* [PATCH 1/8] drm/nouveau/kms/nv50-: Use atomic encoder callbacks everywhere
[not found] <20201114001417.155093-1-lyude@redhat.com>
@ 2020-11-14 0:14 ` Lyude Paul
2020-11-14 4:42 ` [Nouveau] " Ben Skeggs
2020-11-14 0:14 ` [PATCH 2/8] drm/nouveau/kms/nv50-: Remove (nv_encoder->crtc) checks in ->disable callbacks Lyude Paul
` (6 subsequent siblings)
7 siblings, 1 reply; 9+ messages in thread
From: Lyude Paul @ 2020-11-14 0:14 UTC (permalink / raw)
To: dri-devel
Cc: Kirill A . Shutemov, Ben Skeggs, David Airlie, Daniel Vetter,
Dave Airlie, Alex Deucher, Pankaj Bharadiya, Takashi Iwai,
James Jones, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS,
open list
It turns out that I forgot to go through and make sure that I converted all
encoder callbacks to use atomic_enable/atomic_disable(), so let's go and
actually do that.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Fixes: 09838c4efe9a ("drm/nouveau/kms: Search for encoders' connectors properly")
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 29 ++++++++++++-------------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index b111fe24a06b..36d6b6093d16 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -455,7 +455,7 @@ nv50_outp_get_old_connector(struct nouveau_encoder *outp,
* DAC
*****************************************************************************/
static void
-nv50_dac_disable(struct drm_encoder *encoder)
+nv50_dac_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nv50_core *core = nv50_disp(encoder->dev)->core;
@@ -467,7 +467,7 @@ nv50_dac_disable(struct drm_encoder *encoder)
}
static void
-nv50_dac_enable(struct drm_encoder *encoder)
+nv50_dac_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
@@ -525,8 +525,8 @@ nv50_dac_detect(struct drm_encoder *encoder, struct drm_connector *connector)
static const struct drm_encoder_helper_funcs
nv50_dac_help = {
.atomic_check = nv50_outp_atomic_check,
- .enable = nv50_dac_enable,
- .disable = nv50_dac_disable,
+ .atomic_enable = nv50_dac_enable,
+ .atomic_disable = nv50_dac_disable,
.detect = nv50_dac_detect
};
@@ -1055,7 +1055,7 @@ nv50_dp_bpc_to_depth(unsigned int bpc)
}
static void
-nv50_msto_enable(struct drm_encoder *encoder)
+nv50_msto_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nv50_head *head = nv50_head(encoder->crtc);
struct nv50_head_atom *armh = nv50_head_atom(head->base.base.state);
@@ -1101,7 +1101,7 @@ nv50_msto_enable(struct drm_encoder *encoder)
}
static void
-nv50_msto_disable(struct drm_encoder *encoder)
+nv50_msto_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nv50_msto *msto = nv50_msto(encoder);
struct nv50_mstc *mstc = msto->mstc;
@@ -1118,8 +1118,8 @@ nv50_msto_disable(struct drm_encoder *encoder)
static const struct drm_encoder_helper_funcs
nv50_msto_help = {
- .disable = nv50_msto_disable,
- .enable = nv50_msto_enable,
+ .atomic_disable = nv50_msto_disable,
+ .atomic_enable = nv50_msto_enable,
.atomic_check = nv50_msto_atomic_check,
};
@@ -1645,8 +1645,7 @@ nv50_sor_disable(struct drm_encoder *encoder,
}
static void
-nv50_sor_enable(struct drm_encoder *encoder,
- struct drm_atomic_state *state)
+nv50_sor_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
@@ -1873,7 +1872,7 @@ nv50_pior_atomic_check(struct drm_encoder *encoder,
}
static void
-nv50_pior_disable(struct drm_encoder *encoder)
+nv50_pior_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nv50_core *core = nv50_disp(encoder->dev)->core;
@@ -1885,7 +1884,7 @@ nv50_pior_disable(struct drm_encoder *encoder)
}
static void
-nv50_pior_enable(struct drm_encoder *encoder)
+nv50_pior_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
@@ -1921,14 +1920,14 @@ nv50_pior_enable(struct drm_encoder *encoder)
}
core->func->pior->ctrl(core, nv_encoder->or, ctrl, asyh);
- nv_encoder->crtc = encoder->crtc;
+ nv_encoder->crtc = &nv_crtc->base;
}
static const struct drm_encoder_helper_funcs
nv50_pior_help = {
.atomic_check = nv50_pior_atomic_check,
- .enable = nv50_pior_enable,
- .disable = nv50_pior_disable,
+ .atomic_enable = nv50_pior_enable,
+ .atomic_disable = nv50_pior_disable,
};
static void
--
2.28.0
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Nouveau] [PATCH 1/8] drm/nouveau/kms/nv50-: Use atomic encoder callbacks everywhere
2020-11-14 0:14 ` [PATCH 1/8] drm/nouveau/kms/nv50-: Use atomic encoder callbacks everywhere Lyude Paul
@ 2020-11-14 4:42 ` Ben Skeggs
0 siblings, 0 replies; 9+ messages in thread
From: Ben Skeggs @ 2020-11-14 4:42 UTC (permalink / raw)
To: Lyude Paul
Cc: ML dri-devel,
open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS,
David Airlie, Pankaj Bharadiya, open list, Takashi Iwai,
Ben Skeggs, Daniel Vetter, Dave Airlie, Alex Deucher,
Kirill A . Shutemov
I've merged all of these. Sent the first to 5.10-fixes for the
regression there, the rest will go in with a later -next pull request.
Thanks,
Ben.
On Sat, 14 Nov 2020 at 10:14, Lyude Paul <lyude@redhat.com> wrote:
>
> It turns out that I forgot to go through and make sure that I converted all
> encoder callbacks to use atomic_enable/atomic_disable(), so let's go and
> actually do that.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Fixes: 09838c4efe9a ("drm/nouveau/kms: Search for encoders' connectors properly")
> ---
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 29 ++++++++++++-------------
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index b111fe24a06b..36d6b6093d16 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -455,7 +455,7 @@ nv50_outp_get_old_connector(struct nouveau_encoder *outp,
> * DAC
> *****************************************************************************/
> static void
> -nv50_dac_disable(struct drm_encoder *encoder)
> +nv50_dac_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
> {
> struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
> struct nv50_core *core = nv50_disp(encoder->dev)->core;
> @@ -467,7 +467,7 @@ nv50_dac_disable(struct drm_encoder *encoder)
> }
>
> static void
> -nv50_dac_enable(struct drm_encoder *encoder)
> +nv50_dac_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
> {
> struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
> struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
> @@ -525,8 +525,8 @@ nv50_dac_detect(struct drm_encoder *encoder, struct drm_connector *connector)
> static const struct drm_encoder_helper_funcs
> nv50_dac_help = {
> .atomic_check = nv50_outp_atomic_check,
> - .enable = nv50_dac_enable,
> - .disable = nv50_dac_disable,
> + .atomic_enable = nv50_dac_enable,
> + .atomic_disable = nv50_dac_disable,
> .detect = nv50_dac_detect
> };
>
> @@ -1055,7 +1055,7 @@ nv50_dp_bpc_to_depth(unsigned int bpc)
> }
>
> static void
> -nv50_msto_enable(struct drm_encoder *encoder)
> +nv50_msto_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
> {
> struct nv50_head *head = nv50_head(encoder->crtc);
> struct nv50_head_atom *armh = nv50_head_atom(head->base.base.state);
> @@ -1101,7 +1101,7 @@ nv50_msto_enable(struct drm_encoder *encoder)
> }
>
> static void
> -nv50_msto_disable(struct drm_encoder *encoder)
> +nv50_msto_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
> {
> struct nv50_msto *msto = nv50_msto(encoder);
> struct nv50_mstc *mstc = msto->mstc;
> @@ -1118,8 +1118,8 @@ nv50_msto_disable(struct drm_encoder *encoder)
>
> static const struct drm_encoder_helper_funcs
> nv50_msto_help = {
> - .disable = nv50_msto_disable,
> - .enable = nv50_msto_enable,
> + .atomic_disable = nv50_msto_disable,
> + .atomic_enable = nv50_msto_enable,
> .atomic_check = nv50_msto_atomic_check,
> };
>
> @@ -1645,8 +1645,7 @@ nv50_sor_disable(struct drm_encoder *encoder,
> }
>
> static void
> -nv50_sor_enable(struct drm_encoder *encoder,
> - struct drm_atomic_state *state)
> +nv50_sor_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
> {
> struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
> struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
> @@ -1873,7 +1872,7 @@ nv50_pior_atomic_check(struct drm_encoder *encoder,
> }
>
> static void
> -nv50_pior_disable(struct drm_encoder *encoder)
> +nv50_pior_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
> {
> struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
> struct nv50_core *core = nv50_disp(encoder->dev)->core;
> @@ -1885,7 +1884,7 @@ nv50_pior_disable(struct drm_encoder *encoder)
> }
>
> static void
> -nv50_pior_enable(struct drm_encoder *encoder)
> +nv50_pior_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
> {
> struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
> struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
> @@ -1921,14 +1920,14 @@ nv50_pior_enable(struct drm_encoder *encoder)
> }
>
> core->func->pior->ctrl(core, nv_encoder->or, ctrl, asyh);
> - nv_encoder->crtc = encoder->crtc;
> + nv_encoder->crtc = &nv_crtc->base;
> }
>
> static const struct drm_encoder_helper_funcs
> nv50_pior_help = {
> .atomic_check = nv50_pior_atomic_check,
> - .enable = nv50_pior_enable,
> - .disable = nv50_pior_disable,
> + .atomic_enable = nv50_pior_enable,
> + .atomic_disable = nv50_pior_disable,
> };
>
> static void
> --
> 2.28.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/8] drm/nouveau/kms/nv50-: Remove (nv_encoder->crtc) checks in ->disable callbacks
[not found] <20201114001417.155093-1-lyude@redhat.com>
2020-11-14 0:14 ` [PATCH 1/8] drm/nouveau/kms/nv50-: Use atomic encoder callbacks everywhere Lyude Paul
@ 2020-11-14 0:14 ` Lyude Paul
2020-11-14 0:14 ` [PATCH 3/8] drm/nouveau/kms/nv50-: Rename encoder->atomic_(enable|disable) callbacks Lyude Paul
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Lyude Paul @ 2020-11-14 0:14 UTC (permalink / raw)
To: dri-devel
Cc: Ben Skeggs, David Airlie, Daniel Vetter, Dave Airlie,
Pankaj Bharadiya, Alex Deucher, Takashi Iwai, James Jones,
open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list
Noticed these in both the disable (which we'll be getting rid of in a
moment) and the atomic disable callbacks: both callback types check whether
or not there's actually a CRTC assigned to the given encoder. However, as
->atomic_disable and ->disable will never be called without a CRTC assigned
to the given encoder there's no point in this check. So just remove it.
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 37 ++++++++++++-------------
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 36d6b6093d16..073ac66b2922 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -460,8 +460,8 @@ nv50_dac_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nv50_core *core = nv50_disp(encoder->dev)->core;
const u32 ctrl = NVDEF(NV507D, DAC_SET_CONTROL, OWNER, NONE);
- if (nv_encoder->crtc)
- core->func->dac->ctrl(core, nv_encoder->or, ctrl, NULL);
+
+ core->func->dac->ctrl(core, nv_encoder->or, ctrl, NULL);
nv_encoder->crtc = NULL;
nv50_outp_release(nv_encoder);
}
@@ -1620,28 +1620,25 @@ nv50_sor_disable(struct drm_encoder *encoder,
struct nouveau_crtc *nv_crtc = nouveau_crtc(nv_encoder->crtc);
struct nouveau_connector *nv_connector =
nv50_outp_get_old_connector(nv_encoder, state);
+ struct drm_dp_aux *aux = &nv_connector->aux;
+ u8 pwr;
nv_encoder->crtc = NULL;
- if (nv_crtc) {
- struct drm_dp_aux *aux = &nv_connector->aux;
- u8 pwr;
+ if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
+ int ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, &pwr);
- if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
- int ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, &pwr);
-
- if (ret == 0) {
- pwr &= ~DP_SET_POWER_MASK;
- pwr |= DP_SET_POWER_D3;
- drm_dp_dpcd_writeb(aux, DP_SET_POWER, pwr);
- }
+ if (ret == 0) {
+ pwr &= ~DP_SET_POWER_MASK;
+ pwr |= DP_SET_POWER_D3;
+ drm_dp_dpcd_writeb(aux, DP_SET_POWER, pwr);
}
-
- nv_encoder->update(nv_encoder, nv_crtc->index, NULL, 0, 0);
- nv50_audio_disable(encoder, nv_crtc);
- nv50_hdmi_disable(&nv_encoder->base.base, nv_crtc);
- nv50_outp_release(nv_encoder);
}
+
+ nv_encoder->update(nv_encoder, nv_crtc->index, NULL, 0, 0);
+ nv50_audio_disable(encoder, nv_crtc);
+ nv50_hdmi_disable(&nv_encoder->base.base, nv_crtc);
+ nv50_outp_release(nv_encoder);
}
static void
@@ -1877,8 +1874,8 @@ nv50_pior_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nv50_core *core = nv50_disp(encoder->dev)->core;
const u32 ctrl = NVDEF(NV507D, PIOR_SET_CONTROL, OWNER, NONE);
- if (nv_encoder->crtc)
- core->func->pior->ctrl(core, nv_encoder->or, ctrl, NULL);
+
+ core->func->pior->ctrl(core, nv_encoder->or, ctrl, NULL);
nv_encoder->crtc = NULL;
nv50_outp_release(nv_encoder);
}
--
2.28.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 3/8] drm/nouveau/kms/nv50-: Rename encoder->atomic_(enable|disable) callbacks
[not found] <20201114001417.155093-1-lyude@redhat.com>
2020-11-14 0:14 ` [PATCH 1/8] drm/nouveau/kms/nv50-: Use atomic encoder callbacks everywhere Lyude Paul
2020-11-14 0:14 ` [PATCH 2/8] drm/nouveau/kms/nv50-: Remove (nv_encoder->crtc) checks in ->disable callbacks Lyude Paul
@ 2020-11-14 0:14 ` Lyude Paul
2020-11-14 0:14 ` [PATCH 4/8] drm/nouveau/kms/nv50-: s/armh/asyh/ in nv50_msto_atomic_enable() Lyude Paul
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Lyude Paul @ 2020-11-14 0:14 UTC (permalink / raw)
To: dri-devel
Cc: Ben Skeggs, David Airlie, Daniel Vetter, Dave Airlie,
Pankaj Bharadiya, Alex Deucher, Takashi Iwai, James Jones,
open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list
No functional changes, just change the function names to match the
callbacks they provide.
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 33 ++++++++++++-------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 073ac66b2922..a47be145827e 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -455,7 +455,7 @@ nv50_outp_get_old_connector(struct nouveau_encoder *outp,
* DAC
*****************************************************************************/
static void
-nv50_dac_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
+nv50_dac_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nv50_core *core = nv50_disp(encoder->dev)->core;
@@ -467,7 +467,7 @@ nv50_dac_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
}
static void
-nv50_dac_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
+nv50_dac_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
@@ -525,8 +525,8 @@ nv50_dac_detect(struct drm_encoder *encoder, struct drm_connector *connector)
static const struct drm_encoder_helper_funcs
nv50_dac_help = {
.atomic_check = nv50_outp_atomic_check,
- .atomic_enable = nv50_dac_enable,
- .atomic_disable = nv50_dac_disable,
+ .atomic_enable = nv50_dac_atomic_enable,
+ .atomic_disable = nv50_dac_atomic_disable,
.detect = nv50_dac_detect
};
@@ -1055,7 +1055,7 @@ nv50_dp_bpc_to_depth(unsigned int bpc)
}
static void
-nv50_msto_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
+nv50_msto_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nv50_head *head = nv50_head(encoder->crtc);
struct nv50_head_atom *armh = nv50_head_atom(head->base.base.state);
@@ -1101,7 +1101,7 @@ nv50_msto_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
}
static void
-nv50_msto_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
+nv50_msto_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nv50_msto *msto = nv50_msto(encoder);
struct nv50_mstc *mstc = msto->mstc;
@@ -1118,8 +1118,8 @@ nv50_msto_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
static const struct drm_encoder_helper_funcs
nv50_msto_help = {
- .atomic_disable = nv50_msto_disable,
- .atomic_enable = nv50_msto_enable,
+ .atomic_disable = nv50_msto_atomic_disable,
+ .atomic_enable = nv50_msto_atomic_enable,
.atomic_check = nv50_msto_atomic_check,
};
@@ -1613,8 +1613,7 @@ nv50_sor_update(struct nouveau_encoder *nv_encoder, u8 head,
}
static void
-nv50_sor_disable(struct drm_encoder *encoder,
- struct drm_atomic_state *state)
+nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nouveau_crtc *nv_crtc = nouveau_crtc(nv_encoder->crtc);
@@ -1642,7 +1641,7 @@ nv50_sor_disable(struct drm_encoder *encoder,
}
static void
-nv50_sor_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
+nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
@@ -1747,8 +1746,8 @@ nv50_sor_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
static const struct drm_encoder_helper_funcs
nv50_sor_help = {
.atomic_check = nv50_outp_atomic_check,
- .atomic_enable = nv50_sor_enable,
- .atomic_disable = nv50_sor_disable,
+ .atomic_enable = nv50_sor_atomic_enable,
+ .atomic_disable = nv50_sor_atomic_disable,
};
static void
@@ -1869,7 +1868,7 @@ nv50_pior_atomic_check(struct drm_encoder *encoder,
}
static void
-nv50_pior_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
+nv50_pior_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nv50_core *core = nv50_disp(encoder->dev)->core;
@@ -1881,7 +1880,7 @@ nv50_pior_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
}
static void
-nv50_pior_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
+nv50_pior_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
@@ -1923,8 +1922,8 @@ nv50_pior_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
static const struct drm_encoder_helper_funcs
nv50_pior_help = {
.atomic_check = nv50_pior_atomic_check,
- .atomic_enable = nv50_pior_enable,
- .atomic_disable = nv50_pior_disable,
+ .atomic_enable = nv50_pior_atomic_enable,
+ .atomic_disable = nv50_pior_atomic_disable,
};
static void
--
2.28.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 4/8] drm/nouveau/kms/nv50-: s/armh/asyh/ in nv50_msto_atomic_enable()
[not found] <20201114001417.155093-1-lyude@redhat.com>
` (2 preceding siblings ...)
2020-11-14 0:14 ` [PATCH 3/8] drm/nouveau/kms/nv50-: Rename encoder->atomic_(enable|disable) callbacks Lyude Paul
@ 2020-11-14 0:14 ` Lyude Paul
2020-11-14 0:14 ` [PATCH 5/8] drm/nouveau/kms/nv50-: Reverse args for nv50_outp_get_(old|new)_connector() Lyude Paul
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Lyude Paul @ 2020-11-14 0:14 UTC (permalink / raw)
To: dri-devel
Cc: Ben Skeggs, David Airlie, Daniel Vetter, Dave Airlie,
Alex Deucher, Pankaj Bharadiya, Takashi Iwai, James Jones,
open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list
I have a strange dejavu feeling that I tried to submit a patch for this in
the past, but that it was rejected. I can't remember though, but I'm
further convinced this patch is the right thing to do anyway.
We label the to-be-committed head state in nv50_msto_atomic_enable() as
armh. Normally armh implies a state which is currently armed in hardware.
nv50_msto_atomic_enable() is called _after_ drm_atomic_swap_state()
however, but before the commit tail ends, which means that said state is
not actually armed on hardware.
As well - take note that this is the same convention followed in all of the
other atomic_enable() callbacks.
So, let's correct this to asyh.
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index a47be145827e..cbcf3ef517dc 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1058,7 +1058,7 @@ static void
nv50_msto_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nv50_head *head = nv50_head(encoder->crtc);
- struct nv50_head_atom *armh = nv50_head_atom(head->base.base.state);
+ struct nv50_head_atom *asyh = nv50_head_atom(head->base.base.state);
struct nv50_msto *msto = nv50_msto(encoder);
struct nv50_mstc *mstc = NULL;
struct nv50_mstm *mstm = NULL;
@@ -1080,8 +1080,7 @@ nv50_msto_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *st
if (WARN_ON(!mstc))
return;
- r = drm_dp_mst_allocate_vcpi(&mstm->mgr, mstc->port, armh->dp.pbn,
- armh->dp.tu);
+ r = drm_dp_mst_allocate_vcpi(&mstm->mgr, mstc->port, asyh->dp.pbn, asyh->dp.tu);
if (!r)
DRM_DEBUG_KMS("Failed to allocate VCPI\n");
@@ -1093,8 +1092,8 @@ nv50_msto_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *st
else
proto = NV917D_SOR_SET_CONTROL_PROTOCOL_DP_B;
- mstm->outp->update(mstm->outp, head->base.index, armh, proto,
- nv50_dp_bpc_to_depth(armh->or.bpc));
+ mstm->outp->update(mstm->outp, head->base.index, asyh, proto,
+ nv50_dp_bpc_to_depth(asyh->or.bpc));
msto->mstc = mstc;
mstm->modified = true;
--
2.28.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 5/8] drm/nouveau/kms/nv50-: Reverse args for nv50_outp_get_(old|new)_connector()
[not found] <20201114001417.155093-1-lyude@redhat.com>
` (3 preceding siblings ...)
2020-11-14 0:14 ` [PATCH 4/8] drm/nouveau/kms/nv50-: s/armh/asyh/ in nv50_msto_atomic_enable() Lyude Paul
@ 2020-11-14 0:14 ` Lyude Paul
2020-11-14 0:14 ` [PATCH 6/8] drm/nouveau/kms/nv50-: Lookup current encoder/crtc from atomic state Lyude Paul
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Lyude Paul @ 2020-11-14 0:14 UTC (permalink / raw)
To: dri-devel
Cc: Ben Skeggs, David Airlie, Daniel Vetter, Dave Airlie,
Pankaj Bharadiya, Alex Deucher, Takashi Iwai, James Jones,
open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list
Just to be more consistent with the order of args that DRM helpers like
drm_atomic_get_new_crtc_state() use.
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 15 ++++++---------
drivers/gpu/drm/nouveau/nouveau_encoder.h | 6 ++----
2 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index cbcf3ef517dc..2c0749fac9dc 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -418,8 +418,7 @@ nv50_outp_atomic_check(struct drm_encoder *encoder,
}
struct nouveau_connector *
-nv50_outp_get_new_connector(struct nouveau_encoder *outp,
- struct drm_atomic_state *state)
+nv50_outp_get_new_connector(struct drm_atomic_state *state, struct nouveau_encoder *outp)
{
struct drm_connector *connector;
struct drm_connector_state *connector_state;
@@ -435,8 +434,7 @@ nv50_outp_get_new_connector(struct nouveau_encoder *outp,
}
struct nouveau_connector *
-nv50_outp_get_old_connector(struct nouveau_encoder *outp,
- struct drm_atomic_state *state)
+nv50_outp_get_old_connector(struct drm_atomic_state *state, struct nouveau_encoder *outp)
{
struct drm_connector *connector;
struct drm_connector_state *connector_state;
@@ -743,7 +741,7 @@ nv50_audio_enable(struct drm_encoder *encoder, struct drm_atomic_state *state,
(0x0100 << nv_crtc->index),
};
- nv_connector = nv50_outp_get_new_connector(nv_encoder, state);
+ nv_connector = nv50_outp_get_new_connector(state, nv_encoder);
if (!drm_detect_monitor_audio(nv_connector->edid))
return;
@@ -810,7 +808,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_atomic_state *state,
int ret;
int size;
- nv_connector = nv50_outp_get_new_connector(nv_encoder, state);
+ nv_connector = nv50_outp_get_new_connector(state, nv_encoder);
if (!drm_detect_hdmi_monitor(nv_connector->edid))
return;
@@ -1616,8 +1614,7 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *st
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nouveau_crtc *nv_crtc = nouveau_crtc(nv_encoder->crtc);
- struct nouveau_connector *nv_connector =
- nv50_outp_get_old_connector(nv_encoder, state);
+ struct nouveau_connector *nv_connector = nv50_outp_get_old_connector(state, nv_encoder);
struct drm_dp_aux *aux = &nv_connector->aux;
u8 pwr;
@@ -1664,7 +1661,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *sta
u8 proto = NV507D_SOR_SET_CONTROL_PROTOCOL_CUSTOM;
u8 depth = NV837D_SOR_SET_CONTROL_PIXEL_DEPTH_DEFAULT;
- nv_connector = nv50_outp_get_new_connector(nv_encoder, state);
+ nv_connector = nv50_outp_get_new_connector(state, nv_encoder);
nv_encoder->crtc = encoder->crtc;
if ((disp->disp->object.oclass == GT214_DISP ||
diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h b/drivers/gpu/drm/nouveau/nouveau_encoder.h
index 21937f1c7dd9..0dea219a666e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
+++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
@@ -141,11 +141,9 @@ enum drm_mode_status nv50_dp_mode_valid(struct drm_connector *,
unsigned *clock);
struct nouveau_connector *
-nv50_outp_get_new_connector(struct nouveau_encoder *outp,
- struct drm_atomic_state *state);
+nv50_outp_get_new_connector(struct drm_atomic_state *state, struct nouveau_encoder *outp);
struct nouveau_connector *
-nv50_outp_get_old_connector(struct nouveau_encoder *outp,
- struct drm_atomic_state *state);
+nv50_outp_get_old_connector(struct drm_atomic_state *state, struct nouveau_encoder *outp);
int nv50_mstm_detect(struct nouveau_encoder *encoder);
void nv50_mstm_remove(struct nv50_mstm *mstm);
--
2.28.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 6/8] drm/nouveau/kms/nv50-: Lookup current encoder/crtc from atomic state
[not found] <20201114001417.155093-1-lyude@redhat.com>
` (4 preceding siblings ...)
2020-11-14 0:14 ` [PATCH 5/8] drm/nouveau/kms/nv50-: Reverse args for nv50_outp_get_(old|new)_connector() Lyude Paul
@ 2020-11-14 0:14 ` Lyude Paul
2020-11-14 0:14 ` [PATCH 7/8] drm/nouveau/kms/nv50-: Use nouveau_encoder->crtc in get_eld callback Lyude Paul
2020-11-14 0:14 ` [PATCH 8/8] drm/nouveau/kms/nv50-: Fix locking for audio callbacks Lyude Paul
7 siblings, 0 replies; 9+ messages in thread
From: Lyude Paul @ 2020-11-14 0:14 UTC (permalink / raw)
To: dri-devel
Cc: Ben Skeggs, David Airlie, Daniel Vetter, Dave Airlie,
Pankaj Bharadiya, Alex Deucher, Takashi Iwai, James Jones,
open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list
Despite being an atomic driver, nouveau has a lot of leftover code that
relies on retrieving information regarding the new atomic state from
members of drm_encoder and drm_crtc. The first field being used,
drm_encoder.crtc, is deprecated for atomic drivers. The second field being
used is drm_crtc.state, which is only really sensible to use outside of an
atomic modeset.
So, add some helpers to lookup the current crtc for a given outp from the
atomic state. Then, convert most of the code in dispnv50/disp.c to use said
new helper, along with the relevant DRM atomic helpers for retrieving the
new encoder/crtc combinations for a new atomic state.
Note that we don't get rid of the nouveau_encoder.crtc field entirely for
three reasons:
- Legacy modesetting for pre-nv50 still uses it
- It doesn't cause any locking issues
- We need it for the HDA callbacks, as grabbing atomic modesetting locks in
those would be a mess.
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 59 ++++++++++++++++---------
1 file changed, 38 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 2c0749fac9dc..63fff3988f19 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -449,6 +449,22 @@ nv50_outp_get_old_connector(struct drm_atomic_state *state, struct nouveau_encod
return NULL;
}
+static struct nouveau_crtc *
+nv50_outp_get_new_crtc(const struct drm_atomic_state *state, const struct nouveau_encoder *outp)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ const u32 mask = drm_encoder_mask(&outp->base.base);
+ int i;
+
+ for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+ if (crtc_state->encoder_mask & mask)
+ return nouveau_crtc(crtc);
+ }
+
+ return NULL;
+}
+
/******************************************************************************
* DAC
*****************************************************************************/
@@ -468,8 +484,9 @@ static void
nv50_dac_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
- struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
- struct nv50_head_atom *asyh = nv50_head_atom(nv_crtc->base.state);
+ struct nouveau_crtc *nv_crtc = nv50_outp_get_new_crtc(state, nv_encoder);
+ struct nv50_head_atom *asyh =
+ nv50_head_atom(drm_atomic_get_new_crtc_state(state, &nv_crtc->base));
struct nv50_core *core = nv50_disp(encoder->dev)->core;
u32 ctrl = 0;
@@ -490,7 +507,7 @@ nv50_dac_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *sta
core->func->dac->ctrl(core, nv_encoder->or, ctrl, asyh);
asyh->or.depth = 0;
- nv_encoder->crtc = encoder->crtc;
+ nv_encoder->crtc = &nv_crtc->base;
}
static enum drm_connector_status
@@ -719,13 +736,12 @@ nv50_audio_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
}
static void
-nv50_audio_enable(struct drm_encoder *encoder, struct drm_atomic_state *state,
+nv50_audio_enable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc,
+ struct nouveau_connector *nv_connector, struct drm_atomic_state *state,
struct drm_display_mode *mode)
{
struct nouveau_drm *drm = nouveau_drm(encoder->dev);
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
- struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
- struct nouveau_connector *nv_connector;
struct nv50_disp *disp = nv50_disp(encoder->dev);
struct __packed {
struct {
@@ -741,7 +757,6 @@ nv50_audio_enable(struct drm_encoder *encoder, struct drm_atomic_state *state,
(0x0100 << nv_crtc->index),
};
- nv_connector = nv50_outp_get_new_connector(state, nv_encoder);
if (!drm_detect_monitor_audio(nv_connector->edid))
return;
@@ -778,12 +793,12 @@ nv50_hdmi_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
}
static void
-nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_atomic_state *state,
+nv50_hdmi_enable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc,
+ struct nouveau_connector *nv_connector, struct drm_atomic_state *state,
struct drm_display_mode *mode)
{
struct nouveau_drm *drm = nouveau_drm(encoder->dev);
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
- struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
struct nv50_disp *disp = nv50_disp(encoder->dev);
struct {
struct nv50_disp_mthd_v1 base;
@@ -798,7 +813,6 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_atomic_state *state,
.pwr.state = 1,
.pwr.rekey = 56, /* binary driver, and tegra, constant */
};
- struct nouveau_connector *nv_connector;
struct drm_hdmi_info *hdmi;
u32 max_ac_packet;
union hdmi_infoframe avi_frame;
@@ -808,7 +822,6 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_atomic_state *state,
int ret;
int size;
- nv_connector = nv50_outp_get_new_connector(state, nv_encoder);
if (!drm_detect_hdmi_monitor(nv_connector->edid))
return;
@@ -854,7 +867,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_atomic_state *state,
+ args.pwr.vendor_infoframe_length;
nvif_mthd(&disp->disp->object, 0, &args, size);
- nv50_audio_enable(encoder, state, mode);
+ nv50_audio_enable(encoder, nv_crtc, nv_connector, state, mode);
/* If SCDC is supported by the downstream monitor, update
* divider / scrambling settings to what we programmed above.
@@ -895,6 +908,7 @@ struct nv50_mstc {
struct nv50_msto {
struct drm_encoder encoder;
+ /* head is statically assigned on msto creation */
struct nv50_head *head;
struct nv50_mstc *mstc;
bool disabled;
@@ -1055,9 +1069,10 @@ nv50_dp_bpc_to_depth(unsigned int bpc)
static void
nv50_msto_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
- struct nv50_head *head = nv50_head(encoder->crtc);
- struct nv50_head_atom *asyh = nv50_head_atom(head->base.base.state);
struct nv50_msto *msto = nv50_msto(encoder);
+ struct nv50_head *head = msto->head;
+ struct nv50_head_atom *asyh =
+ nv50_head_atom(drm_atomic_get_new_crtc_state(state, &head->base.base));
struct nv50_mstc *mstc = NULL;
struct nv50_mstm *mstm = NULL;
struct drm_connector *connector;
@@ -1640,8 +1655,9 @@ static void
nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
- struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
- struct nv50_head_atom *asyh = nv50_head_atom(nv_crtc->base.state);
+ struct nouveau_crtc *nv_crtc = nv50_outp_get_new_crtc(state, nv_encoder);
+ struct nv50_head_atom *asyh =
+ nv50_head_atom(drm_atomic_get_new_crtc_state(state, &nv_crtc->base));
struct drm_display_mode *mode = &asyh->state.adjusted_mode;
struct {
struct nv50_disp_mthd_v1 base;
@@ -1662,7 +1678,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *sta
u8 depth = NV837D_SOR_SET_CONTROL_PIXEL_DEPTH_DEFAULT;
nv_connector = nv50_outp_get_new_connector(state, nv_encoder);
- nv_encoder->crtc = encoder->crtc;
+ nv_encoder->crtc = &nv_crtc->base;
if ((disp->disp->object.oclass == GT214_DISP ||
disp->disp->object.oclass >= GF110_DISP) &&
@@ -1688,7 +1704,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *sta
proto = NV507D_SOR_SET_CONTROL_PROTOCOL_SINGLE_TMDS_B;
}
- nv50_hdmi_enable(&nv_encoder->base.base, state, mode);
+ nv50_hdmi_enable(&nv_encoder->base.base, nv_crtc, nv_connector, state, mode);
break;
case DCB_OUTPUT_LVDS:
proto = NV507D_SOR_SET_CONTROL_PROTOCOL_LVDS_CUSTOM;
@@ -1729,7 +1745,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *sta
else
proto = NV887D_SOR_SET_CONTROL_PROTOCOL_DP_B;
- nv50_audio_enable(encoder, state, mode);
+ nv50_audio_enable(encoder, nv_crtc, nv_connector, state, mode);
break;
default:
BUG();
@@ -1879,8 +1895,9 @@ static void
nv50_pior_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
- struct nouveau_crtc *nv_crtc = nouveau_crtc(encoder->crtc);
- struct nv50_head_atom *asyh = nv50_head_atom(nv_crtc->base.state);
+ struct nouveau_crtc *nv_crtc = nv50_outp_get_new_crtc(state, nv_encoder);
+ struct nv50_head_atom *asyh =
+ nv50_head_atom(drm_atomic_get_new_crtc_state(state, &nv_crtc->base));
struct nv50_core *core = nv50_disp(encoder->dev)->core;
u32 ctrl = 0;
--
2.28.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 7/8] drm/nouveau/kms/nv50-: Use nouveau_encoder->crtc in get_eld callback
[not found] <20201114001417.155093-1-lyude@redhat.com>
` (5 preceding siblings ...)
2020-11-14 0:14 ` [PATCH 6/8] drm/nouveau/kms/nv50-: Lookup current encoder/crtc from atomic state Lyude Paul
@ 2020-11-14 0:14 ` Lyude Paul
2020-11-14 0:14 ` [PATCH 8/8] drm/nouveau/kms/nv50-: Fix locking for audio callbacks Lyude Paul
7 siblings, 0 replies; 9+ messages in thread
From: Lyude Paul @ 2020-11-14 0:14 UTC (permalink / raw)
To: dri-devel
Cc: Ben Skeggs, David Airlie, Daniel Vetter, Dave Airlie,
Alex Deucher, Pankaj Bharadiya, Takashi Iwai, James Jones,
open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list
drm_encoder->crtc is deprecated for atomic drivers, but
nouveau_encoder->crtc is safe.
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 63fff3988f19..b19d0d7a6de9 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -630,7 +630,7 @@ nv50_audio_component_get_eld(struct device *kdev, int port, int dev_id,
if (!nv_connector)
continue;
- nv_crtc = nouveau_crtc(encoder->crtc);
+ nv_crtc = nouveau_crtc(nv_encoder->crtc);
if (!nv_crtc || nv_encoder->or != port ||
nv_crtc->index != dev_id)
continue;
--
2.28.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 8/8] drm/nouveau/kms/nv50-: Fix locking for audio callbacks
[not found] <20201114001417.155093-1-lyude@redhat.com>
` (6 preceding siblings ...)
2020-11-14 0:14 ` [PATCH 7/8] drm/nouveau/kms/nv50-: Use nouveau_encoder->crtc in get_eld callback Lyude Paul
@ 2020-11-14 0:14 ` Lyude Paul
7 siblings, 0 replies; 9+ messages in thread
From: Lyude Paul @ 2020-11-14 0:14 UTC (permalink / raw)
To: dri-devel
Cc: Ben Skeggs, David Airlie, Daniel Vetter, Dave Airlie,
Alex Deucher, Pankaj Bharadiya, Takashi Iwai, James Jones,
open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list
Noticed that I wasn't paying close enough attention the last time I looked
at our audio callbacks, as I completely missed the fact that we were
figuring out which audio-enabled connector goes to each encoder by checking
it's state, but without grabbing any of the appropriate modesetting locks
to do so.
That being said however: trying to grab modesetting locks in our audio
callbacks would be very painful due to the potential for locking inversion
between HDA and DRM. So, let's instead just copy what i915 does again - add
our own audio lock to protect audio related state, and store each audio
enabled connector in each nouveau_encoder struct so that we don't need to
check any atomic states.
Signed-off-by: Lyude Paul <lyude@redhat.com>
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 66 ++++++++++++-----------
drivers/gpu/drm/nouveau/nouveau_drv.h | 1 +
drivers/gpu/drm/nouveau/nouveau_encoder.h | 7 ++-
3 files changed, 43 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index b19d0d7a6de9..65a991846e96 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -607,34 +607,27 @@ nv50_audio_component_get_eld(struct device *kdev, int port, int dev_id,
struct nouveau_drm *drm = nouveau_drm(drm_dev);
struct drm_encoder *encoder;
struct nouveau_encoder *nv_encoder;
- struct drm_connector *connector;
struct nouveau_crtc *nv_crtc;
- struct drm_connector_list_iter conn_iter;
int ret = 0;
*enabled = false;
+ mutex_lock(&drm->audio.lock);
+
drm_for_each_encoder(encoder, drm->dev) {
struct nouveau_connector *nv_connector = NULL;
+ if (encoder->encoder_type == DRM_MODE_ENCODER_DPMST)
+ continue; /* TODO */
+
nv_encoder = nouveau_encoder(encoder);
+ nv_connector = nouveau_connector(nv_encoder->audio.connector);
+ nv_crtc = nouveau_crtc(nv_encoder->crtc);
- drm_connector_list_iter_begin(drm_dev, &conn_iter);
- drm_for_each_connector_iter(connector, &conn_iter) {
- if (connector->state->best_encoder == encoder) {
- nv_connector = nouveau_connector(connector);
- break;
- }
- }
- drm_connector_list_iter_end(&conn_iter);
- if (!nv_connector)
+ if (!nv_crtc || nv_encoder->or != port || nv_crtc->index != dev_id)
continue;
- nv_crtc = nouveau_crtc(nv_encoder->crtc);
- if (!nv_crtc || nv_encoder->or != port ||
- nv_crtc->index != dev_id)
- continue;
- *enabled = nv_encoder->audio;
+ *enabled = nv_encoder->audio.enabled;
if (*enabled) {
ret = drm_eld_size(nv_connector->base.eld);
memcpy(buf, nv_connector->base.eld,
@@ -643,6 +636,8 @@ nv50_audio_component_get_eld(struct device *kdev, int port, int dev_id,
break;
}
+ mutex_unlock(&drm->audio.lock);
+
return ret;
}
@@ -692,17 +687,22 @@ static const struct component_ops nv50_audio_component_bind_ops = {
static void
nv50_audio_component_init(struct nouveau_drm *drm)
{
- if (!component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
- drm->audio.component_registered = true;
+ if (component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
+ return;
+
+ drm->audio.component_registered = true;
+ mutex_init(&drm->audio.lock);
}
static void
nv50_audio_component_fini(struct nouveau_drm *drm)
{
- if (drm->audio.component_registered) {
- component_del(drm->dev->dev, &nv50_audio_component_bind_ops);
- drm->audio.component_registered = false;
- }
+ if (!drm->audio.component_registered)
+ return;
+
+ component_del(drm->dev->dev, &nv50_audio_component_bind_ops);
+ drm->audio.component_registered = false;
+ mutex_destroy(&drm->audio.lock);
}
/******************************************************************************
@@ -725,11 +725,13 @@ nv50_audio_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
(0x0100 << nv_crtc->index),
};
- if (!nv_encoder->audio)
- return;
-
- nv_encoder->audio = false;
- nvif_mthd(&disp->disp->object, 0, &args, sizeof(args));
+ mutex_lock(&drm->audio.lock);
+ if (nv_encoder->audio.enabled) {
+ nv_encoder->audio.enabled = false;
+ nv_encoder->audio.connector = NULL;
+ nvif_mthd(&disp->disp->object, 0, &args, sizeof(args));
+ }
+ mutex_unlock(&drm->audio.lock);
nv50_audio_component_eld_notify(drm->audio.component, nv_encoder->or,
nv_crtc->index);
@@ -760,11 +762,16 @@ nv50_audio_enable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc,
if (!drm_detect_monitor_audio(nv_connector->edid))
return;
+ mutex_lock(&drm->audio.lock);
+
memcpy(args.data, nv_connector->base.eld, sizeof(args.data));
nvif_mthd(&disp->disp->object, 0, &args,
sizeof(args.base) + drm_eld_size(args.data));
- nv_encoder->audio = true;
+ nv_encoder->audio.enabled = true;
+ nv_encoder->audio.connector = &nv_connector->base;
+
+ mutex_unlock(&drm->audio.lock);
nv50_audio_component_eld_notify(drm->audio.component, nv_encoder->or,
nv_crtc->index);
@@ -1633,8 +1640,6 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *st
struct drm_dp_aux *aux = &nv_connector->aux;
u8 pwr;
- nv_encoder->crtc = NULL;
-
if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
int ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, &pwr);
@@ -1649,6 +1654,7 @@ nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *st
nv50_audio_disable(encoder, nv_crtc);
nv50_hdmi_disable(&nv_encoder->base.base, nv_crtc);
nv50_outp_release(nv_encoder);
+ nv_encoder->crtc = NULL;
}
static void
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index 9d04d1b36434..d91bf9c78976 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -222,6 +222,7 @@ struct nouveau_drm {
struct {
struct drm_audio_component *component;
+ struct mutex lock;
bool component_registered;
} audio;
};
diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h b/drivers/gpu/drm/nouveau/nouveau_encoder.h
index 0dea219a666e..1ffcc0a491fd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
+++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
@@ -53,7 +53,12 @@ struct nouveau_encoder {
* actually programmed on the hw, not the proposed crtc */
struct drm_crtc *crtc;
u32 ctrl;
- bool audio;
+
+ /* Protected by nouveau_drm.audio.lock */
+ struct {
+ bool enabled;
+ struct drm_connector *connector;
+ } audio;
struct drm_display_mode mode;
int last_dpms;
--
2.28.0
^ permalink raw reply [flat|nested] 9+ messages in thread