Re: [Intel-gfx] [PATCH v8 06/11] drm/i915: Enable big joiner support in enable and disable sequences.
Op 28-08-2020 om 01:35 schreef Navare, Manasi:
> On Mon, Aug 10, 2020 at 04:28:28PM -0700, Manasi Navare wrote:
>> From: Maarten Lankhorst
>>
>> Make vdsc work when no output is enabled. The big joiner needs VDSC
>> on the slave, so enable it and set the appropriate bits.
>> Also update timestamping constants, because slave crtc's are not
>> updated in drm_atomic_helper_update_legacy_modeset_state().
>>
>> This should be enough to bring up CRTC's in a big joiner configuration,
>> without any plane configuration on the second pipe yet.
>>
>> HOWEVER, we still bring up the crtc's in the wrong order. We need to
>> make sure that the master crtc is brought up after the slave crtc.
>> This is done correctly later in this series.
>>
>> The next steps are to enable planes correctly, and make sure we enable
>> and update both master and slave in the correct order.
>>
>> v2:
>> * Manual rebase (Manasi)
>>
>> v3:
>> * Rebase (Manasi)
>>
>> v4:
>> * Rebase (Manasi)
>>
>> v5:
>> * Get dsc power domain in ddi_init (Manasi)
>>
>> v6:
>> * Remove dsc power put from dsc_disable (Maarten)
>>
>> Signed-off-by: Maarten Lankhorst
>> Signed-off-by: Manasi Navare
>> ---
>> drivers/gpu/drm/i915/display/icl_dsi.c| 2 -
>> drivers/gpu/drm/i915/display/intel_ddi.c | 68 +++-
>> drivers/gpu/drm/i915/display/intel_display.c | 377 --
>> .../drm/i915/display/intel_display_types.h| 1 +
>> drivers/gpu/drm/i915/display/intel_dp.c | 6 +-
>> drivers/gpu/drm/i915/display/intel_vdsc.c | 201 +-
>> drivers/gpu/drm/i915/display/intel_vdsc.h | 7 +-
>> 7 files changed, 413 insertions(+), 249 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>> b/drivers/gpu/drm/i915/display/icl_dsi.c
>> index 8c55f5bee9ab..26f7372b4c25 100644
>> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> @@ -1454,8 +1454,6 @@ static void gen11_dsi_get_config(struct intel_encoder
>> *encoder,
>> struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
>> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>
>> -intel_dsc_get_config(encoder, pipe_config);
> Maarten,
> Why do we need to remove this from dsi_get_config()?
This is read by the pipe now, which is the only place that does get_config().
>
>> -
>> /* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
>> pipe_config->port_clock = intel_dpll_get_freq(i915,
>>pipe_config->shared_dpll);
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>> b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index de5b216561d8..6de13c67f5b8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -28,6 +28,7 @@
>> #include
>>
>> #include "i915_drv.h"
>> +#include "i915_trace.h"
>> #include "intel_audio.h"
>> #include "intel_combo_phy.h"
>> #include "intel_connector.h"
>> @@ -2093,12 +2094,6 @@ static void intel_ddi_get_power_domains(struct
>> intel_encoder *encoder,
>> intel_display_power_get(dev_priv,
>>
>> intel_ddi_main_link_aux_domain(dig_port));
>>
>> -/*
>> - * VDSC power is needed when DSC is enabled
>> - */
>> -if (crtc_state->dsc.compression_enable)
>> -intel_display_power_get(dev_priv,
>> -intel_dsc_power_domain(crtc_state));
>> }
>>
>> void intel_ddi_enable_pipe_clock(struct intel_encoder *encoder,
>> @@ -3387,7 +3382,8 @@ static void tgl_ddi_pre_enable_dp(struct
>> intel_atomic_state *state,
>>
>> /* 7.l Configure and enable FEC if needed */
>> intel_ddi_enable_fec(encoder, crtc_state);
>> -intel_dsc_enable(encoder, crtc_state);
>> +if (!crtc_state->bigjoiner)
>> +intel_dsc_enable(encoder, crtc_state);
>> }
>>
>> static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>> @@ -3458,7 +3454,8 @@ static void hsw_ddi_pre_enable_dp(struct
>> intel_atomic_state *state,
>> if (!is_mst)
>> intel_ddi_enable_pipe_clock(encoder, crtc_state);
>>
>> -intel_dsc_enable(encoder, crtc_state);
>> +if (!crtc_state->bigjoiner)
>> +intel_dsc_enable(encoder, crtc_state);
>> }
>>
>> static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
>> @@ -3713,6 +3710,21 @@ static void intel_ddi_post_disable(struct
>> intel_atomic_state *state,
>> ilk_pfit_disable(old_crtc_state);
>> }
>>
>> +if (old_crtc_state->bigjoiner_linked_crtc) {
>> +struct intel_atomic_state *state =
>> +to_intel_atomic_state(old_crtc_state->uapi.state);
>> +struct intel_crtc *slave =
>> +old_crtc_state->bigjoiner_linked_crtc;
>> +const struct intel_crtc_state *old_slave_crtc_state =
>> +intel_atomic_get_old_crtc_state(state, slave);
Re: [Intel-gfx] [PATCH v8 06/11] drm/i915: Enable big joiner support in enable and disable sequences.
On Mon, Aug 10, 2020 at 04:28:28PM -0700, Manasi Navare wrote:
> From: Maarten Lankhorst
>
> Make vdsc work when no output is enabled. The big joiner needs VDSC
> on the slave, so enable it and set the appropriate bits.
> Also update timestamping constants, because slave crtc's are not
> updated in drm_atomic_helper_update_legacy_modeset_state().
>
> This should be enough to bring up CRTC's in a big joiner configuration,
> without any plane configuration on the second pipe yet.
>
> HOWEVER, we still bring up the crtc's in the wrong order. We need to
> make sure that the master crtc is brought up after the slave crtc.
> This is done correctly later in this series.
>
> The next steps are to enable planes correctly, and make sure we enable
> and update both master and slave in the correct order.
>
> v2:
> * Manual rebase (Manasi)
>
> v3:
> * Rebase (Manasi)
>
> v4:
> * Rebase (Manasi)
>
> v5:
> * Get dsc power domain in ddi_init (Manasi)
>
> v6:
> * Remove dsc power put from dsc_disable (Maarten)
>
> Signed-off-by: Maarten Lankhorst
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c| 2 -
> drivers/gpu/drm/i915/display/intel_ddi.c | 68 +++-
> drivers/gpu/drm/i915/display/intel_display.c | 377 --
> .../drm/i915/display/intel_display_types.h| 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 6 +-
> drivers/gpu/drm/i915/display/intel_vdsc.c | 201 +-
> drivers/gpu/drm/i915/display/intel_vdsc.h | 7 +-
> 7 files changed, 413 insertions(+), 249 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 8c55f5bee9ab..26f7372b4c25 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1454,8 +1454,6 @@ static void gen11_dsi_get_config(struct intel_encoder
> *encoder,
> struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>
> - intel_dsc_get_config(encoder, pipe_config);
Maarten,
Why do we need to remove this from dsi_get_config()?
> -
> /* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
> pipe_config->port_clock = intel_dpll_get_freq(i915,
> pipe_config->shared_dpll);
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index de5b216561d8..6de13c67f5b8 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -28,6 +28,7 @@
> #include
>
> #include "i915_drv.h"
> +#include "i915_trace.h"
> #include "intel_audio.h"
> #include "intel_combo_phy.h"
> #include "intel_connector.h"
> @@ -2093,12 +2094,6 @@ static void intel_ddi_get_power_domains(struct
> intel_encoder *encoder,
> intel_display_power_get(dev_priv,
>
> intel_ddi_main_link_aux_domain(dig_port));
>
> - /*
> - * VDSC power is needed when DSC is enabled
> - */
> - if (crtc_state->dsc.compression_enable)
> - intel_display_power_get(dev_priv,
> - intel_dsc_power_domain(crtc_state));
> }
>
> void intel_ddi_enable_pipe_clock(struct intel_encoder *encoder,
> @@ -3387,7 +3382,8 @@ static void tgl_ddi_pre_enable_dp(struct
> intel_atomic_state *state,
>
> /* 7.l Configure and enable FEC if needed */
> intel_ddi_enable_fec(encoder, crtc_state);
> - intel_dsc_enable(encoder, crtc_state);
> + if (!crtc_state->bigjoiner)
> + intel_dsc_enable(encoder, crtc_state);
> }
>
> static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
> @@ -3458,7 +3454,8 @@ static void hsw_ddi_pre_enable_dp(struct
> intel_atomic_state *state,
> if (!is_mst)
> intel_ddi_enable_pipe_clock(encoder, crtc_state);
>
> - intel_dsc_enable(encoder, crtc_state);
> + if (!crtc_state->bigjoiner)
> + intel_dsc_enable(encoder, crtc_state);
> }
>
> static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
> @@ -3713,6 +3710,21 @@ static void intel_ddi_post_disable(struct
> intel_atomic_state *state,
> ilk_pfit_disable(old_crtc_state);
> }
>
> + if (old_crtc_state->bigjoiner_linked_crtc) {
> + struct intel_atomic_state *state =
> + to_intel_atomic_state(old_crtc_state->uapi.state);
> + struct intel_crtc *slave =
> + old_crtc_state->bigjoiner_linked_crtc;
> + const struct intel_crtc_state *old_slave_crtc_state =
> + intel_atomic_get_old_crtc_state(state, slave);
> +
> + intel_crtc_vblank_off(old_slave_crtc_state);
> + trace_intel_pipe_disable(slave);
> +
> + intel_dsc_disable(old_slave_crtc_state);
> + skl_scaler_disab
[Intel-gfx] [PATCH v8 06/11] drm/i915: Enable big joiner support in enable and disable sequences.
From: Maarten Lankhorst
Make vdsc work when no output is enabled. The big joiner needs VDSC
on the slave, so enable it and set the appropriate bits.
Also update timestamping constants, because slave crtc's are not
updated in drm_atomic_helper_update_legacy_modeset_state().
This should be enough to bring up CRTC's in a big joiner configuration,
without any plane configuration on the second pipe yet.
HOWEVER, we still bring up the crtc's in the wrong order. We need to
make sure that the master crtc is brought up after the slave crtc.
This is done correctly later in this series.
The next steps are to enable planes correctly, and make sure we enable
and update both master and slave in the correct order.
v2:
* Manual rebase (Manasi)
v3:
* Rebase (Manasi)
v4:
* Rebase (Manasi)
v5:
* Get dsc power domain in ddi_init (Manasi)
v6:
* Remove dsc power put from dsc_disable (Maarten)
Signed-off-by: Maarten Lankhorst
Signed-off-by: Manasi Navare
---
drivers/gpu/drm/i915/display/icl_dsi.c| 2 -
drivers/gpu/drm/i915/display/intel_ddi.c | 68 +++-
drivers/gpu/drm/i915/display/intel_display.c | 377 --
.../drm/i915/display/intel_display_types.h| 1 +
drivers/gpu/drm/i915/display/intel_dp.c | 6 +-
drivers/gpu/drm/i915/display/intel_vdsc.c | 201 +-
drivers/gpu/drm/i915/display/intel_vdsc.h | 7 +-
7 files changed, 413 insertions(+), 249 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
b/drivers/gpu/drm/i915/display/icl_dsi.c
index 8c55f5bee9ab..26f7372b4c25 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1454,8 +1454,6 @@ static void gen11_dsi_get_config(struct intel_encoder
*encoder,
struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
- intel_dsc_get_config(encoder, pipe_config);
-
/* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
pipe_config->port_clock = intel_dpll_get_freq(i915,
pipe_config->shared_dpll);
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index de5b216561d8..6de13c67f5b8 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -28,6 +28,7 @@
#include
#include "i915_drv.h"
+#include "i915_trace.h"
#include "intel_audio.h"
#include "intel_combo_phy.h"
#include "intel_connector.h"
@@ -2093,12 +2094,6 @@ static void intel_ddi_get_power_domains(struct
intel_encoder *encoder,
intel_display_power_get(dev_priv,
intel_ddi_main_link_aux_domain(dig_port));
- /*
-* VDSC power is needed when DSC is enabled
-*/
- if (crtc_state->dsc.compression_enable)
- intel_display_power_get(dev_priv,
- intel_dsc_power_domain(crtc_state));
}
void intel_ddi_enable_pipe_clock(struct intel_encoder *encoder,
@@ -3387,7 +3382,8 @@ static void tgl_ddi_pre_enable_dp(struct
intel_atomic_state *state,
/* 7.l Configure and enable FEC if needed */
intel_ddi_enable_fec(encoder, crtc_state);
- intel_dsc_enable(encoder, crtc_state);
+ if (!crtc_state->bigjoiner)
+ intel_dsc_enable(encoder, crtc_state);
}
static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
@@ -3458,7 +3454,8 @@ static void hsw_ddi_pre_enable_dp(struct
intel_atomic_state *state,
if (!is_mst)
intel_ddi_enable_pipe_clock(encoder, crtc_state);
- intel_dsc_enable(encoder, crtc_state);
+ if (!crtc_state->bigjoiner)
+ intel_dsc_enable(encoder, crtc_state);
}
static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
@@ -3713,6 +3710,21 @@ static void intel_ddi_post_disable(struct
intel_atomic_state *state,
ilk_pfit_disable(old_crtc_state);
}
+ if (old_crtc_state->bigjoiner_linked_crtc) {
+ struct intel_atomic_state *state =
+ to_intel_atomic_state(old_crtc_state->uapi.state);
+ struct intel_crtc *slave =
+ old_crtc_state->bigjoiner_linked_crtc;
+ const struct intel_crtc_state *old_slave_crtc_state =
+ intel_atomic_get_old_crtc_state(state, slave);
+
+ intel_crtc_vblank_off(old_slave_crtc_state);
+ trace_intel_pipe_disable(slave);
+
+ intel_dsc_disable(old_slave_crtc_state);
+ skl_scaler_disable(old_slave_crtc_state);
+ }
+
/*
* When called from DP MST code:
* - old_conn_state will be NULL
@@ -3927,7 +3939,8 @@ static void intel_enable_ddi(struct intel_atomic_state
*state,
{
drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
- intel_ddi_
