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 <[email protected]> > Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for MST") > Cc: Ben Skeggs <[email protected]> > Cc: Daniel Vetter <[email protected]> > Cc: David Airlie <[email protected]> > Cc: Jerry Zuo <[email protected]> > Cc: Harry Wentland <[email protected]> > Cc: Juston Li <[email protected]> > Cc: Sean Paul <[email protected]> > Cc: Laurent Pinchart <[email protected]> > Cc: <[email protected]> # 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 <[email protected]>
> +}
> +
> 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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/nouveau
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
