Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init
Hi Ville,
You means this change right? Sure. Will try your suggestion as well.
By the way, what is different between hw.mode and uapi.mode and how we know
which to be used? It used to only base.mode before hw/uapi split patches.
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14671,8 +14671,8 @@ static int intel_atomic_check(struct drm_device *dev,
> /* Catch I915_MODE_FLAG_INHERITED */
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> - if (new_crtc_state->hw.mode.private_flags !=
> - old_crtc_state->hw.mode.private_flags)
> + if (new_crtc_state->uapi.mode.private_flags !=
> + old_crtc_state->uapi.mode.private_flags)
> new_crtc_state->uapi.mode_changed = true;
> }
>
> ?
Regards,
SweeAun
-Original Message-
From: Ville Syrjälä
Sent: Friday, March 20, 2020 11:24 PM
To: Shankar, Uma
Cc: Souza, Jose ; intel-gfx@lists.freedesktop.org; Khor,
Swee Aun
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for
audio codec init
On Fri, Mar 20, 2020 at 06:19:37AM +, Shankar, Uma wrote:
>
>
> > -Original Message-
> > From: Souza, Jose
> > Sent: Friday, March 20, 2020 12:36 AM
> > To: Shankar, Uma ;
> > intel-gfx@lists.freedesktop.org
> > Cc: Khor, Swee Aun
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset
> > at boot for audio codec init
> >
> > On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> > > If external monitors are connected during boot up, driver uses the
> > > same mode programmed by BIOS and avoids a full modeset.
> > > This results in display audio codec left uninitialized and display
> > > audio fails to work till user triggers a modeset.
> > >
> > > This patch fixes the same by triggering a modeset at boot.
> >
> > We had the same issue for PSR, take a look to the fix:
> > commit 33e059a2e4df454359f642f2235af39de9d3e914
> > drm/i915/psr: Force PSR probe only after full initialization
> >
> > Maybe make this even more generic.
>
> Yeah this looks to dealing with almost a similar need. Thanks for
> pointing this out, will try to come up with a generalized solution.
How about just fixing the uapi vs. hw fail I showed instead of adding even more
hacks?
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init
Hi Ville,
You are right, your suggestion will fix this issue.
#Based on dmesg log, uapi mode private flags change is captured
...
[ 11.404578] fbcon: i915drmfb (fb0) is primary device
[ 11.404743] [drm] SA: intel_atomic_check: uapi change
[ 11.404744] [drm] SA2: intel_atomic_check:
new_crtc_state->uapi.mode.private_flags= 0,
old_crtc_state->uapi.mode.private_flags= 1
[ 11.404744] [drm] SA2: intel_atomic_check:
new_crtc_state->uapi.mode.private_flags= 0,
old_crtc_state->uapi.mode.private_flags= 0
[ 11.404745] [drm] SA2: intel_atomic_check:
new_crtc_state->uapi.mode.private_flags= 0,
old_crtc_state->uapi.mode.private_flags= 0
[ 11.404799] [drm:intel_atomic_check [i915]] [CONNECTOR:110:HDMI-A-2]
Limiting display bpp to 24 instead of EDID bpp 24, requested bpp 36, max
platform bpp 36
[ 11.404855] [drm:intel_hdmi_compute_config [i915]] picking 8 bpc for HDMI
output (pipe bpp: 24)
[ 11.404898] [drm:intel_atomic_check [i915]] hw max bpp: 24, pipe bpp: 24,
dithering: 0
...
#Here is the git diff
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 4d1634ed6a1b..b5c56cd513d9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14108,11 +14108,15 @@ static int intel_atomic_check(struct drm_device *dev,
int ret, i;
bool any_ms = false;
+ DRM_INFO("SA: intel_atomic_check: uapi change \n");
+
/* Catch I915_MODE_FLAG_INHERITED */
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
- if (new_crtc_state->hw.mode.private_flags !=
- old_crtc_state->hw.mode.private_flags)
+
+ DRM_INFO("SA2: intel_atomic_check:
new_crtc_state->uapi.mode.private_flags= %d,
old_crtc_state->uapi.mode.private_flags= %d \n",
new_crtc_state->uapi.mode.private_flags,
old_crtc_state->uapi.mode.private_flags );
+ if (new_crtc_state->uapi.mode.private_flags !=
+ old_crtc_state->uapi.mode.private_flags)
new_crtc_state->uapi.mode_changed = true;
}
Regards,
SweeAun
-Original Message-
From: Khor, Swee Aun
Sent: Saturday, March 21, 2020 12:55 AM
To: Ville Syrjälä ; Shankar, Uma
Cc: Souza, Jose ; intel-gfx@lists.freedesktop.org
Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for
audio codec init
Hi Ville,
You means this change right? Sure. Will try your suggestion as well.
By the way, what is different between hw.mode and uapi.mode and how we know
which to be used? It used to only base.mode before hw/uapi split patches.
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14671,8 +14671,8 @@ static int intel_atomic_check(struct drm_device *dev,
> /* Catch I915_MODE_FLAG_INHERITED */
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> - if (new_crtc_state->hw.mode.private_flags !=
> - old_crtc_state->hw.mode.private_flags)
> + if (new_crtc_state->uapi.mode.private_flags !=
> + old_crtc_state->uapi.mode.private_flags)
> new_crtc_state->uapi.mode_changed = true;
> }
>
> ?
Regards,
SweeAun
-Original Message-
From: Ville Syrjälä
Sent: Friday, March 20, 2020 11:24 PM
To: Shankar, Uma
Cc: Souza, Jose ; intel-gfx@lists.freedesktop.org; Khor,
Swee Aun
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for
audio codec init
On Fri, Mar 20, 2020 at 06:19:37AM +, Shankar, Uma wrote:
>
>
> > -Original Message-
> > From: Souza, Jose
> > Sent: Friday, March 20, 2020 12:36 AM
> > To: Shankar, Uma ;
> > intel-gfx@lists.freedesktop.org
> > Cc: Khor, Swee Aun
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset
> > at boot for audio codec init
> >
> > On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> > > If external monitors are connected during boot up, driver uses the
> > > same mode programmed by BIOS and avoids a full modeset.
> > > This results in display audio codec left uninitialized and display
> > > audio fails to work till user triggers a modeset.
> > >
> > > This patch fixes the same by triggering a modeset at boot.
> >
> > We had the same issue for PSR, take a look to the fix:
> > commit 33e059a2e4df454359f642f2235af39de9d3e914
> > drm/i915/psr: Force PSR probe only after full initialization
> >
> > Maybe make
Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init
Git diff without debug print. Please review. Thanks.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 4d1634ed6a1b..806cf622fb39 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14108,11 +14108,13 @@ static int intel_atomic_check(struct drm_device *dev,
int ret, i;
bool any_ms = false;
+
/* Catch I915_MODE_FLAG_INHERITED */
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
- if (new_crtc_state->hw.mode.private_flags !=
- old_crtc_state->hw.mode.private_flags)
+
+ if (new_crtc_state->uapi.mode.private_flags !=
+ old_crtc_state->uapi.mode.private_flags)
new_crtc_state->uapi.mode_changed = true;
}
Regards,
SweeAun
-Original Message-----
From: Khor, Swee Aun
Sent: Monday, March 23, 2020 10:29 PM
To: 'Ville Syrjälä' ; Shankar, Uma
Cc: Souza, Jose ; 'intel-gfx@lists.freedesktop.org'
Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for
audio codec init
Hi Ville,
You are right, your suggestion will fix this issue.
#Based on dmesg log, uapi mode private flags change is captured ...
[ 11.404578] fbcon: i915drmfb (fb0) is primary device
[ 11.404743] [drm] SA: intel_atomic_check: uapi change
[ 11.404744] [drm] SA2: intel_atomic_check:
new_crtc_state->uapi.mode.private_flags= 0,
old_crtc_state->uapi.mode.private_flags= 1
[ 11.404744] [drm] SA2: intel_atomic_check:
new_crtc_state->uapi.mode.private_flags= 0,
old_crtc_state->uapi.mode.private_flags= 0
[ 11.404745] [drm] SA2: intel_atomic_check:
new_crtc_state->uapi.mode.private_flags= 0,
old_crtc_state->uapi.mode.private_flags= 0
[ 11.404799] [drm:intel_atomic_check [i915]] [CONNECTOR:110:HDMI-A-2]
Limiting display bpp to 24 instead of EDID bpp 24, requested bpp 36, max
platform bpp 36
[ 11.404855] [drm:intel_hdmi_compute_config [i915]] picking 8 bpc for HDMI
output (pipe bpp: 24)
[ 11.404898] [drm:intel_atomic_check [i915]] hw max bpp: 24, pipe bpp: 24,
dithering: 0
...
#Here is the git diff
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 4d1634ed6a1b..b5c56cd513d9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14108,11 +14108,15 @@ static int intel_atomic_check(struct drm_device *dev,
int ret, i;
bool any_ms = false;
+ DRM_INFO("SA: intel_atomic_check: uapi change \n");
+
/* Catch I915_MODE_FLAG_INHERITED */
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
- if (new_crtc_state->hw.mode.private_flags !=
- old_crtc_state->hw.mode.private_flags)
+
+ DRM_INFO("SA2: intel_atomic_check:
new_crtc_state->uapi.mode.private_flags= %d,
old_crtc_state->uapi.mode.private_flags= %d \n",
new_crtc_state->uapi.mode.private_flags,
old_crtc_state->uapi.mode.private_flags );
+ if (new_crtc_state->uapi.mode.private_flags !=
+ old_crtc_state->uapi.mode.private_flags)
new_crtc_state->uapi.mode_changed = true;
}
Regards,
SweeAun
-Original Message-
From: Khor, Swee Aun
Sent: Saturday, March 21, 2020 12:55 AM
To: Ville Syrjälä ; Shankar, Uma
Cc: Souza, Jose ; intel-gfx@lists.freedesktop.org
Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for
audio codec init
Hi Ville,
You means this change right? Sure. Will try your suggestion as well.
By the way, what is different between hw.mode and uapi.mode and how we know
which to be used? It used to only base.mode before hw/uapi split patches.
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14671,8 +14671,8 @@ static int intel_atomic_check(struct drm_device *dev,
> /* Catch I915_MODE_FLAG_INHERITED */
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> - if (new_crtc_state->hw.mode.private_flags !=
> - old_crtc_state->hw.mode.private_flags)
> + if (new_crtc_state->uapi.mode.private_flags !=
> + old_crtc_state->uapi.mode.private_flags)
> new_crtc_state->uapi.mode_changed = true;
> }
>
> ?
Regards,
SweeAun
-----Original Message-
From: Ville Syrjälä
Sent: Friday, March 20, 2020 11:24 PM
To: Shankar, Uma
Cc: Souza, Jose ; intel
Re: [Intel-gfx] [PATCH 11/23] drm/i915/rkl: Add cdclk support
Hi Matt,
The follow cdclk doesn't looked right, isn’t it should be 96000 and 54
according to their respective divider and ratio?
+ { .refclk = 19200, .cdclk = 192000, .divider = 3, .ratio = 15 },
+ { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 45 },
Regards,
SweeAun
-Original Message-
From: Intel-gfx On Behalf Of Matt
Roper
Sent: Saturday, May 2, 2020 1:08 AM
To: intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH 11/23] drm/i915/rkl: Add cdclk support
Note that the 192000 clock frequencies can be achieved with different pairs of
ratio+divider, which is something we haven't encountered before. If any of
those ratios were common with other legal cdclk values, then it would mean we
could avoid triggering full modesets if we just needed to change the divider.
However at the moment there don't appear to be any valid cdclks that share the
same ratio so we can't take advantage of this and it doesn't really matter
which approach we use to achieve the 192000 cdclk. For now our driver
functions that operate on the table will just always pick the first entry
(lower ratio + lower divider).
Bspec: 49202
Cc: Ville Syrjälä
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 54 +++---
1 file changed, 48 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 979a0241fdcb..4ca87260e8ba 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1230,6 +1230,40 @@ static const struct intel_cdclk_vals icl_cdclk_table[] =
{
{}
};
+/*
+ * RKL has multiple divider+ratio pairs that can hit cdclk=192000. Our
+ * functions to read these tables will just always pick the first one
+which
+ * should be fine since there's no other valid cdclk value that can be
+achieved
+ * via the same ratio with a different divider (i.e., no opportunity to
+avoid a
+ * full modeset).
+ */
+static const struct intel_cdclk_vals rkl_cdclk_table[] = {
+ { .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 },
+ { .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 },
+ { .refclk = 19200, .cdclk = 192000, .divider = 3, .ratio = 15 },
+ { .refclk = 19200, .cdclk = 307200, .divider = 2, .ratio = 32 },
+ { .refclk = 19200, .cdclk = 326400, .divider = 4, .ratio = 68 },
+ { .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 },
+ { .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 },
+
+ { .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 },
+ { .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 },
+ { .refclk = 24000, .cdclk = 192000, .divider = 3, .ratio = 24 },
+ { .refclk = 24000, .cdclk = 312000, .divider = 2, .ratio = 26 },
+ { .refclk = 24000, .cdclk = 324000, .divider = 4, .ratio = 54 },
+ { .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 45 },
+ { .refclk = 24000, .cdclk = 648000, .divider = 2, .ratio = 54 },
+
+ { .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 },
+ { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 },
+ { .refclk = 38400, .cdclk = 192000, .divider = 3, .ratio = 15 },
+ { .refclk = 38400, .cdclk = 307200, .divider = 2, .ratio = 16 },
+ { .refclk = 38400, .cdclk = 326400, .divider = 4, .ratio = 34 },
+ { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 },
+ { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 },
+ {}
+};
+
static int bxt_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk) {
const struct intel_cdclk_vals *table = dev_priv->cdclk.table; @@
-1405,8 +1439,8 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
div = 2;
break;
case BXT_CDCLK_CD2X_DIV_SEL_1_5:
- drm_WARN(&dev_priv->drm,
-IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10,
+ drm_WARN(&dev_priv->drm, IS_GEMINILAKE(dev_priv) ||
+(INTEL_GEN(dev_priv) >= 10 &&
!IS_ROCKETLAKE(dev_priv)),
"Unsupported divider\n");
div = 3;
break;
@@ -1414,7 +1448,8 @@ static void bxt_get_cdclk(struct drm_i915_private
*dev_priv,
div = 4;
break;
case BXT_CDCLK_CD2X_DIV_SEL_4:
- drm_WARN(&dev_priv->drm, INTEL_GEN(dev_priv) >= 10,
+ drm_WARN(&dev_priv->drm,
+INTEL_GEN(dev_priv) >= 10 && !IS_ROCKETLAKE(dev_priv),
"Unsupported divider\n");
div = 8;
break;
@@ -1564,7 +1599,8 @@ static void bxt_set_cdclk(struct drm_i915_private
*dev_priv,
break;
case 3:
drm_WARN(&dev_priv->drm,
-
