Skip to content

Commit

Permalink
drm/nouveau/kms: Search for encoders' connectors properly
Browse files Browse the repository at this point in the history
While the way we find the associated connector for an encoder is just
fine for legacy modesetting, it's not correct for nv50+ since that uses
atomic modesetting. For reference, see the drm_encoder kdocs.

Fix this by removing nouveau_encoder_connector_get(), and replacing it
with nv04_encoder_get_connector(), nv50_outp_get_old_connector(), and
nv50_outp_get_new_connector().

v2:
* Don't line-wrap for_each_(old|new)_connector_in_state in
  nv50_outp_get_(old|new)_connector() - sravn
v3:
* Fix potential uninitialized usage of nv_connector (needs to be
  initialized to NULL at the start). Thanks kernel test robot!
v4:
* Actually fix uninitialized nv_connector usage in
  nv50_audio_component_get_eld(). The previous fix wouldn't have worked
  since we would have started out with nv_connector == NULL, but
  wouldn't clear it after a single drm_for_each_encoder() iteration.
  Thanks again Kernel bot!

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Ben Skeggs <bskeggs@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200826182456.322681-7-lyude@redhat.com
  • Loading branch information
Lyude Paul committed Aug 31, 2020
1 parent 254e7e3 commit 09838c4
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 38 deletions.
2 changes: 1 addition & 1 deletion drivers/gpu/drm/nouveau/dispnv04/dac.c
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ static void nv04_dac_commit(struct drm_encoder *encoder)
helper->dpms(encoder, DRM_MODE_DPMS_ON);

NV_DEBUG(drm, "Output %s is running on CRTC %d using output %c\n",
nouveau_encoder_connector_get(nv_encoder)->base.name,
nv04_encoder_get_connector(nv_encoder)->base.name,
nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
}

Expand Down
7 changes: 4 additions & 3 deletions drivers/gpu/drm/nouveau/dispnv04/dfp.c
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ static bool nv04_dfp_mode_fixup(struct drm_encoder *encoder,
struct drm_display_mode *adjusted_mode)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
struct nouveau_connector *nv_connector = nouveau_encoder_connector_get(nv_encoder);
struct nouveau_connector *nv_connector =
nv04_encoder_get_connector(nv_encoder);

if (!nv_connector->native_mode ||
nv_connector->scaling_mode == DRM_MODE_SCALE_NONE ||
Expand Down Expand Up @@ -478,7 +479,7 @@ static void nv04_dfp_commit(struct drm_encoder *encoder)
helper->dpms(encoder, DRM_MODE_DPMS_ON);

NV_DEBUG(drm, "Output %s is running on CRTC %d using output %c\n",
nouveau_encoder_connector_get(nv_encoder)->base.name,
nv04_encoder_get_connector(nv_encoder)->base.name,
nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
}

Expand Down Expand Up @@ -591,7 +592,7 @@ static void nv04_dfp_restore(struct drm_encoder *encoder)

if (nv_encoder->dcb->type == DCB_OUTPUT_LVDS) {
struct nouveau_connector *connector =
nouveau_encoder_connector_get(nv_encoder);
nv04_encoder_get_connector(nv_encoder);

if (connector && connector->native_mode)
call_lvds_script(dev, nv_encoder->dcb, head,
Expand Down
18 changes: 18 additions & 0 deletions drivers/gpu/drm/nouveau/dispnv04/disp.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@

#include <nvif/if0004.h>

struct nouveau_connector *
nv04_encoder_get_connector(struct nouveau_encoder *encoder)
{
struct drm_device *dev = to_drm_encoder(encoder)->dev;
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
struct nouveau_connector *nv_connector = NULL;

drm_connector_list_iter_begin(dev, &conn_iter);
drm_for_each_connector_iter(connector, &conn_iter) {
if (connector->encoder == to_drm_encoder(encoder))
nv_connector = nouveau_connector(connector);
}
drm_connector_list_iter_end(&conn_iter);

return nv_connector;
}

static void
nv04_display_fini(struct drm_device *dev, bool suspend)
{
Expand Down
4 changes: 4 additions & 0 deletions drivers/gpu/drm/nouveau/dispnv04/disp.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#include "nouveau_display.h"

struct nouveau_encoder;

enum nv04_fp_display_regs {
FP_DISPLAY_END,
FP_TOTAL,
Expand Down Expand Up @@ -93,6 +95,8 @@ nv04_display(struct drm_device *dev)

/* nv04_display.c */
int nv04_display_create(struct drm_device *);
struct nouveau_connector *
nv04_encoder_get_connector(struct nouveau_encoder *nv_encoder);

/* nv04_crtc.c */
int nv04_crtc_create(struct drm_device *, int index);
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ static void nv04_tv_commit(struct drm_encoder *encoder)
helper->dpms(encoder, DRM_MODE_DPMS_ON);

NV_DEBUG(drm, "Output %s is running on CRTC %d using output %c\n",
nouveau_encoder_connector_get(nv_encoder)->base.name,
nv04_encoder_get_connector(nv_encoder)->base.name,
nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
}

Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ static void nv17_tv_commit(struct drm_encoder *encoder)
helper->dpms(encoder, DRM_MODE_DPMS_ON);

NV_INFO(drm, "Output %s is running on CRTC %d using output %c\n",
nouveau_encoder_connector_get(nv_encoder)->base.name,
nv04_encoder_get_connector(nv_encoder)->base.name,
nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
}

Expand Down
88 changes: 71 additions & 17 deletions drivers/gpu/drm/nouveau/dispnv50/disp.c
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,40 @@ nv50_outp_atomic_check(struct drm_encoder *encoder,
return 0;
}

struct nouveau_connector *
nv50_outp_get_new_connector(struct nouveau_encoder *outp,
struct drm_atomic_state *state)
{
struct drm_connector *connector;
struct drm_connector_state *connector_state;
struct drm_encoder *encoder = to_drm_encoder(outp);
int i;

for_each_new_connector_in_state(state, connector, connector_state, i) {
if (connector_state->best_encoder == encoder)
return nouveau_connector(connector);
}

return NULL;
}

struct nouveau_connector *
nv50_outp_get_old_connector(struct nouveau_encoder *outp,
struct drm_atomic_state *state)
{
struct drm_connector *connector;
struct drm_connector_state *connector_state;
struct drm_encoder *encoder = to_drm_encoder(outp);
int i;

for_each_old_connector_in_state(state, connector, connector_state, i) {
if (connector_state->best_encoder == encoder)
return nouveau_connector(connector);
}

return NULL;
}

/******************************************************************************
* DAC
*****************************************************************************/
Expand Down Expand Up @@ -552,16 +586,31 @@ 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 nouveau_connector *nv_connector;
struct drm_connector *connector;
struct nouveau_crtc *nv_crtc;
struct drm_connector_list_iter conn_iter;
int ret = 0;

*enabled = false;

drm_for_each_encoder(encoder, drm->dev) {
struct nouveau_connector *nv_connector = NULL;

nv_encoder = nouveau_encoder(encoder);
nv_connector = nouveau_encoder_connector_get(nv_encoder);

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)
continue;

nv_crtc = nouveau_crtc(encoder->crtc);
if (!nv_connector || !nv_crtc || nv_encoder->or != port ||
if (!nv_crtc || nv_encoder->or != port ||
nv_crtc->index != dev_id)
continue;
*enabled = nv_encoder->audio;
Expand All @@ -572,6 +621,7 @@ nv50_audio_component_get_eld(struct device *kdev, int port, int dev_id,
}
break;
}

return ret;
}

Expand Down Expand Up @@ -665,7 +715,8 @@ nv50_audio_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
}

static void
nv50_audio_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
nv50_audio_enable(struct drm_encoder *encoder, 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);
Expand All @@ -686,7 +737,7 @@ nv50_audio_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
(0x0100 << nv_crtc->index),
};

nv_connector = nouveau_encoder_connector_get(nv_encoder);
nv_connector = nv50_outp_get_new_connector(nv_encoder, state);
if (!drm_detect_monitor_audio(nv_connector->edid))
return;

Expand Down Expand Up @@ -723,7 +774,8 @@ nv50_hdmi_disable(struct drm_encoder *encoder, struct nouveau_crtc *nv_crtc)
}

static void
nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
nv50_hdmi_enable(struct drm_encoder *encoder, 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);
Expand Down Expand Up @@ -752,7 +804,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
int ret;
int size;

nv_connector = nouveau_encoder_connector_get(nv_encoder);
nv_connector = nv50_outp_get_new_connector(nv_encoder, state);
if (!drm_detect_hdmi_monitor(nv_connector->edid))
return;

Expand Down Expand Up @@ -798,7 +850,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
+ args.pwr.vendor_infoframe_length;
nvif_mthd(&disp->disp->object, 0, &args, size);

nv50_audio_enable(encoder, mode);
nv50_audio_enable(encoder, state, mode);

/* If SCDC is supported by the downstream monitor, update
* divider / scrambling settings to what we programmed above.
Expand Down Expand Up @@ -1573,7 +1625,8 @@ nv50_sor_update(struct nouveau_encoder *nv_encoder, u8 head,
}

static void
nv50_sor_disable(struct drm_encoder *encoder)
nv50_sor_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);
Expand Down Expand Up @@ -1601,7 +1654,8 @@ nv50_sor_disable(struct drm_encoder *encoder)
}

static void
nv50_sor_enable(struct drm_encoder *encoder)
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);
Expand All @@ -1625,7 +1679,7 @@ nv50_sor_enable(struct drm_encoder *encoder)
u8 proto = NV507D_SOR_SET_CONTROL_PROTOCOL_CUSTOM;
u8 depth = NV837D_SOR_SET_CONTROL_PIXEL_DEPTH_DEFAULT;

nv_connector = nouveau_encoder_connector_get(nv_encoder);
nv_connector = nv50_outp_get_new_connector(nv_encoder, state);
nv_encoder->crtc = encoder->crtc;

if ((disp->disp->object.oclass == GT214_DISP ||
Expand All @@ -1652,7 +1706,7 @@ nv50_sor_enable(struct drm_encoder *encoder)
proto = NV507D_SOR_SET_CONTROL_PROTOCOL_SINGLE_TMDS_B;
}

nv50_hdmi_enable(&nv_encoder->base.base, mode);
nv50_hdmi_enable(&nv_encoder->base.base, state, mode);
break;
case DCB_OUTPUT_LVDS:
proto = NV507D_SOR_SET_CONTROL_PROTOCOL_LVDS_CUSTOM;
Expand Down Expand Up @@ -1693,7 +1747,7 @@ nv50_sor_enable(struct drm_encoder *encoder)
else
proto = NV887D_SOR_SET_CONTROL_PROTOCOL_DP_B;

nv50_audio_enable(encoder, mode);
nv50_audio_enable(encoder, state, mode);
break;
default:
BUG();
Expand All @@ -1706,8 +1760,8 @@ nv50_sor_enable(struct drm_encoder *encoder)
static const struct drm_encoder_helper_funcs
nv50_sor_help = {
.atomic_check = nv50_outp_atomic_check,
.enable = nv50_sor_enable,
.disable = nv50_sor_disable,
.atomic_enable = nv50_sor_enable,
.atomic_disable = nv50_sor_disable,
};

static void
Expand Down Expand Up @@ -2066,7 +2120,7 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state *state)
outp->clr.mask, outp->set.mask);

if (outp->clr.mask) {
help->disable(encoder);
help->atomic_disable(encoder, state);
interlock[NV50_DISP_INTERLOCK_CORE] |= 1;
if (outp->flush_disable) {
nv50_disp_atomic_commit_wndw(state, interlock);
Expand Down Expand Up @@ -2105,7 +2159,7 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state *state)
outp->set.mask, outp->clr.mask);

if (outp->set.mask) {
help->enable(encoder);
help->atomic_enable(encoder, state);
interlock[NV50_DISP_INTERLOCK_CORE] = 1;
}

Expand Down
14 changes: 0 additions & 14 deletions drivers/gpu/drm/nouveau/nouveau_connector.c
Original file line number Diff line number Diff line change
Expand Up @@ -391,20 +391,6 @@ find_encoder(struct drm_connector *connector, int type)
return NULL;
}

struct nouveau_connector *
nouveau_encoder_connector_get(struct nouveau_encoder *encoder)
{
struct drm_device *dev = to_drm_encoder(encoder)->dev;
struct drm_connector *drm_connector;

list_for_each_entry(drm_connector, &dev->mode_config.connector_list, head) {
if (drm_connector->encoder == to_drm_encoder(encoder))
return nouveau_connector(drm_connector);
}

return NULL;
}

static void
nouveau_connector_destroy(struct drm_connector *connector)
{
Expand Down
6 changes: 5 additions & 1 deletion drivers/gpu/drm/nouveau/nouveau_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,11 @@ enum drm_mode_status nv50_dp_mode_valid(struct drm_connector *,
unsigned *clock);

struct nouveau_connector *
nouveau_encoder_connector_get(struct nouveau_encoder *encoder);
nv50_outp_get_new_connector(struct nouveau_encoder *outp,
struct drm_atomic_state *state);
struct nouveau_connector *
nv50_outp_get_old_connector(struct nouveau_encoder *outp,
struct drm_atomic_state *state);

int nv50_mstm_detect(struct nv50_mstm *, u8 dpcd[8], int allow);
void nv50_mstm_remove(struct nv50_mstm *);
Expand Down

0 comments on commit 09838c4

Please sign in to comment.