| Message ID | 20191115210728.3467-3-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MST BPC fixes for nouveau | expand |
On Fri, Nov 15, 2019 at 04:07:19PM -0500, Lyude Paul wrote: > In order to be able to use bpc values that are different from what the > connector reports, we want to be able to store the bpc value we decide > on using for an atomic state in nv50_head_atom and refer to that instead > of simply using the value that the connector reports throughout the > whole atomic check phase and commit phase. This will let us (eventually) > implement the max bpc connector property, and will also be needed for > limiting the bpc we use on MST displays to 8 in the next commit. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for MST") > Cc: Ben Skeggs <bskeggs@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: David Airlie <airlied@redhat.com> > Cc: Jerry Zuo <Jerry.Zuo@amd.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Juston Li <juston.li@intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: <stable@vger.kernel.org> # v5.1+ > --- > drivers/gpu/drm/nouveau/dispnv50/atom.h | 1 + > drivers/gpu/drm/nouveau/dispnv50/disp.c | 57 ++++++++++++++----------- > drivers/gpu/drm/nouveau/dispnv50/head.c | 5 +-- > 3 files changed, 36 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/atom.h b/drivers/gpu/drm/nouveau/dispnv50/atom.h > index 43df86c38f58..24f7700768da 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/atom.h > +++ b/drivers/gpu/drm/nouveau/dispnv50/atom.h > @@ -114,6 +114,7 @@ struct nv50_head_atom { > u8 nhsync:1; > u8 nvsync:1; > u8 depth:4; > + u8 bpc; > } or; > > /* Currently only used for MST */ > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index 6327aaf37c08..93665aecce57 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -353,10 +353,20 @@ nv50_outp_atomic_check(struct drm_encoder *encoder, > struct drm_crtc_state *crtc_state, > struct drm_connector_state *conn_state) > { > - struct nouveau_connector *nv_connector = > - nouveau_connector(conn_state->connector); > - return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state, > - nv_connector->native_mode); > + struct drm_connector *connector = conn_state->connector; > + struct nouveau_connector *nv_connector = nouveau_connector(connector); > + struct nv50_head_atom *asyh = nv50_head_atom(crtc_state); > + int ret; > + > + ret = nv50_outp_atomic_check_view(encoder, crtc_state, conn_state, > + nv_connector->native_mode); > + if (ret) > + return ret; > + > + if (crtc_state->mode_changed || crtc_state->connectors_changed) > + asyh->or.bpc = connector->display_info.bpc; > + > + return 0; > } > > /****************************************************************************** > @@ -786,10 +796,10 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, > * may have changed after the state was duplicated > */ > if (!state->duplicated) { > - const int bpp = connector->display_info.bpc * 3; > const int clock = crtc_state->adjusted_mode.clock; > > - asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp); > + asyh->or.bpc = connector->display_info.bpc; > + asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3); > } > > slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr, mstc->port, > @@ -802,6 +812,17 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, > return 0; > } > > +static u8 > +nv50_dp_bpc_to_depth(unsigned int bpc) > +{ > + switch (bpc) { > + case 6: return 0x2; > + case 8: return 0x5; > + case 10: /* fall-through */ > + default: return 0x6; > + } This is obviously just refactored from the code below, so this is probably fine for now. But what about BPC > 10? The OR here seems to be very similar to what's used on Tegra where the same values are used in the SOR_STATE1 register, see: drivers/gpu/drm/tegra/sor.h There are additional values for 12 and 16 BPC (see the definitions for SOR_STATE_ASY_PIXELDEPTH_BPP_*). With the above anything higher than 10 BPC will be treated the same and likely lead to wrong results. So I think either a WARN for the "default" case or additional cases for the other values would be good to have. Like I said, if this even is problematic (and given that userspace does not really support > 8 BPC yet, it probably isn't) it's a preexisting problem, so can be done in a different patch. Other than that this looks good, so: Reviewed-by: Thierry Reding <treding@nvidia.com> > +} > + > static void > nv50_msto_enable(struct drm_encoder *encoder) > { > @@ -812,7 +833,7 @@ nv50_msto_enable(struct drm_encoder *encoder) > struct nv50_mstm *mstm = NULL; > struct drm_connector *connector; > struct drm_connector_list_iter conn_iter; > - u8 proto, depth; > + u8 proto; > bool r; > > drm_connector_list_iter_begin(encoder->dev, &conn_iter); > @@ -841,14 +862,8 @@ nv50_msto_enable(struct drm_encoder *encoder) > else > proto = 0x9; > > - switch (mstc->connector.display_info.bpc) { > - case 6: depth = 0x2; break; > - case 8: depth = 0x5; break; > - case 10: > - default: depth = 0x6; break; > - } > - > - mstm->outp->update(mstm->outp, head->base.index, armh, proto, depth); > + mstm->outp->update(mstm->outp, head->base.index, armh, proto, > + nv50_dp_bpc_to_depth(armh->or.bpc)); > > msto->head = head; > msto->mstc = mstc; > @@ -1502,20 +1517,14 @@ nv50_sor_enable(struct drm_encoder *encoder) > lvds.lvds.script |= 0x0200; > } > > - if (nv_connector->base.display_info.bpc == 8) > + if (asyh->or.bpc == 8) > lvds.lvds.script |= 0x0200; > } > > nvif_mthd(&disp->disp->object, 0, &lvds, sizeof(lvds)); > break; > case DCB_OUTPUT_DP: > - if (nv_connector->base.display_info.bpc == 6) > - depth = 0x2; > - else > - if (nv_connector->base.display_info.bpc == 8) > - depth = 0x5; > - else > - depth = 0x6; > + depth = nv50_dp_bpc_to_depth(asyh->or.bpc); > > if (nv_encoder->link & 1) > proto = 0x8; > @@ -1666,7 +1675,7 @@ nv50_pior_enable(struct drm_encoder *encoder) > nv50_outp_acquire(nv_encoder); > > nv_connector = nouveau_encoder_connector_get(nv_encoder); > - switch (nv_connector->base.display_info.bpc) { > + switch (asyh->or.bpc) { > case 10: asyh->or.depth = 0x6; break; > case 8: asyh->or.depth = 0x5; break; > case 6: asyh->or.depth = 0x2; break; > diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c > index 71c23bf1fe25..c9692df2b76c 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/head.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c > @@ -81,18 +81,17 @@ nv50_head_atomic_check_dither(struct nv50_head_atom *armh, > struct nv50_head_atom *asyh, > struct nouveau_conn_atom *asyc) > { > - struct drm_connector *connector = asyc->state.connector; > u32 mode = 0x00; > > if (asyc->dither.mode == DITHERING_MODE_AUTO) { > - if (asyh->base.depth > connector->display_info.bpc * 3) > + if (asyh->base.depth > asyh->or.bpc * 3) > mode = DITHERING_MODE_DYNAMIC2X2; > } else { > mode = asyc->dither.mode; > } > > if (asyc->dither.depth == DITHERING_DEPTH_AUTO) { > - if (connector->display_info.bpc >= 8) > + if (asyh->or.bpc >= 8) > mode |= DITHERING_DEPTH_8BPC; > } else { > mode |= asyc->dither.depth; > -- > 2.21.0 > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
diff --git a/drivers/gpu/drm/nouveau/dispnv50/atom.h b/drivers/gpu/drm/nouveau/dispnv50/atom.h index 43df86c38f58..24f7700768da 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/atom.h +++ b/drivers/gpu/drm/nouveau/dispnv50/atom.h @@ -114,6 +114,7 @@ struct nv50_head_atom { u8 nhsync:1; u8 nvsync:1; u8 depth:4; + u8 bpc; } or; /* Currently only used for MST */ diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 6327aaf37c08..93665aecce57 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -353,10 +353,20 @@ nv50_outp_atomic_check(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { - struct nouveau_connector *nv_connector = - nouveau_connector(conn_state->connector); - return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state, - nv_connector->native_mode); + struct drm_connector *connector = conn_state->connector; + struct nouveau_connector *nv_connector = nouveau_connector(connector); + struct nv50_head_atom *asyh = nv50_head_atom(crtc_state); + int ret; + + ret = nv50_outp_atomic_check_view(encoder, crtc_state, conn_state, + nv_connector->native_mode); + if (ret) + return ret; + + if (crtc_state->mode_changed || crtc_state->connectors_changed) + asyh->or.bpc = connector->display_info.bpc; + + return 0; } /****************************************************************************** @@ -786,10 +796,10 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, * may have changed after the state was duplicated */ if (!state->duplicated) { - const int bpp = connector->display_info.bpc * 3; const int clock = crtc_state->adjusted_mode.clock; - asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp); + asyh->or.bpc = connector->display_info.bpc; + asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3); } slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr, mstc->port, @@ -802,6 +812,17 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, return 0; } +static u8 +nv50_dp_bpc_to_depth(unsigned int bpc) +{ + switch (bpc) { + case 6: return 0x2; + case 8: return 0x5; + case 10: /* fall-through */ + default: return 0x6; + } +} + static void nv50_msto_enable(struct drm_encoder *encoder) { @@ -812,7 +833,7 @@ nv50_msto_enable(struct drm_encoder *encoder) struct nv50_mstm *mstm = NULL; struct drm_connector *connector; struct drm_connector_list_iter conn_iter; - u8 proto, depth; + u8 proto; bool r; drm_connector_list_iter_begin(encoder->dev, &conn_iter); @@ -841,14 +862,8 @@ nv50_msto_enable(struct drm_encoder *encoder) else proto = 0x9; - switch (mstc->connector.display_info.bpc) { - case 6: depth = 0x2; break; - case 8: depth = 0x5; break; - case 10: - default: depth = 0x6; break; - } - - mstm->outp->update(mstm->outp, head->base.index, armh, proto, depth); + mstm->outp->update(mstm->outp, head->base.index, armh, proto, + nv50_dp_bpc_to_depth(armh->or.bpc)); msto->head = head; msto->mstc = mstc; @@ -1502,20 +1517,14 @@ nv50_sor_enable(struct drm_encoder *encoder) lvds.lvds.script |= 0x0200; } - if (nv_connector->base.display_info.bpc == 8) + if (asyh->or.bpc == 8) lvds.lvds.script |= 0x0200; } nvif_mthd(&disp->disp->object, 0, &lvds, sizeof(lvds)); break; case DCB_OUTPUT_DP: - if (nv_connector->base.display_info.bpc == 6) - depth = 0x2; - else - if (nv_connector->base.display_info.bpc == 8) - depth = 0x5; - else - depth = 0x6; + depth = nv50_dp_bpc_to_depth(asyh->or.bpc); if (nv_encoder->link & 1) proto = 0x8; @@ -1666,7 +1675,7 @@ nv50_pior_enable(struct drm_encoder *encoder) nv50_outp_acquire(nv_encoder); nv_connector = nouveau_encoder_connector_get(nv_encoder); - switch (nv_connector->base.display_info.bpc) { + switch (asyh->or.bpc) { case 10: asyh->or.depth = 0x6; break; case 8: asyh->or.depth = 0x5; break; case 6: asyh->or.depth = 0x2; break; diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c index 71c23bf1fe25..c9692df2b76c 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/head.c +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c @@ -81,18 +81,17 @@ nv50_head_atomic_check_dither(struct nv50_head_atom *armh, struct nv50_head_atom *asyh, struct nouveau_conn_atom *asyc) { - struct drm_connector *connector = asyc->state.connector; u32 mode = 0x00; if (asyc->dither.mode == DITHERING_MODE_AUTO) { - if (asyh->base.depth > connector->display_info.bpc * 3) + if (asyh->base.depth > asyh->or.bpc * 3) mode = DITHERING_MODE_DYNAMIC2X2; } else { mode = asyc->dither.mode; } if (asyc->dither.depth == DITHERING_DEPTH_AUTO) { - if (connector->display_info.bpc >= 8) + if (asyh->or.bpc >= 8) mode |= DITHERING_DEPTH_8BPC; } else { mode |= asyc->dither.depth;
In order to be able to use bpc values that are different from what the connector reports, we want to be able to store the bpc value we decide on using for an atomic state in nv50_head_atom and refer to that instead of simply using the value that the connector reports throughout the whole atomic check phase and commit phase. This will let us (eventually) implement the max bpc connector property, and will also be needed for limiting the bpc we use on MST displays to 8 in the next commit. Signed-off-by: Lyude Paul <lyude@redhat.com> Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for MST") Cc: Ben Skeggs <bskeggs@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: David Airlie <airlied@redhat.com> Cc: Jerry Zuo <Jerry.Zuo@amd.com> Cc: Harry Wentland <harry.wentland@amd.com> Cc: Juston Li <juston.li@intel.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: <stable@vger.kernel.org> # v5.1+ --- drivers/gpu/drm/nouveau/dispnv50/atom.h | 1 + drivers/gpu/drm/nouveau/dispnv50/disp.c | 57 ++++++++++++++----------- drivers/gpu/drm/nouveau/dispnv50/head.c | 5 +-- 3 files changed, 36 insertions(+), 27 deletions(-)