[Intel-gfx] [PATCH 4/5] Add support for forcing 6 bpc on DP pipes.
Ander Conselvan De Oliveira
conselvan2 at gmail.com
Thu May 26 09:00:59 UTC 2016
On Wed, 2016-05-25 at 17:42 -0700, Manasi Navare wrote:
> On Tue, May 24, 2016 at 08:45:45AM +0300, Ander Conselvan De Oliveira wrote:
> >
> > On Mon, 2016-05-23 at 10:42 -0700, Jim Bride wrote:
> > >
> > > On Mon, May 23, 2016 at 11:22:17AM +0300, Ander Conselvan De Oliveira
> > > wrote:
> > > >
> > > >
> > > > On Fri, 2016-04-29 at 18:28 -0700, Manasi Navare wrote:
> > > > >
> > > > >
> > > > > From: Jim Bride <jim.bride at linux.intel.com>
> > > > >
> > > > > For DP compliance we need to be able to control the output color
> > > > > type for the pipe associated with the DP port. To do this we rely
> > > > > on the intel_dp_test_force_bpc debugfs file and the associated value
> > > > > stored in struct intel_dp. If the debugfs file has a non-zero value
> > > > > and we're on a display port connector, then we use the value from
> > > > > debugfs to calculate the bpp for the pipe. For cases where we are
> > > > > not on DP or there has not been an overridden value then we behave
> > > > > as normal.
> > > > >
> > > > > Signed-off-by: Jim Bride <jim.bride at linux.intel.com>
> > > > > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_display.c | 32
> > > > > ++++++++++++++++++++++++++++++-
> > > > > -
> > > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > > 2 files changed, 31 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > index 5ffccf6..1618d36 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -12102,11 +12102,39 @@ compute_baseline_pipe_bpp(struct intel_crtc
> > > > > *crtc,
> > > > >
> > > > > /* Clamp display bpp to EDID value */
> > > > > for_each_connector_in_state(state, connector,
> > > > > connector_state, i)
> > > > > {
> > > > > + int type = 0;
> > > > > +
> > > > > + if (connector_state->best_encoder) {
> > > > > + struct intel_encoder *ienc;
> > > > > +
> > > > > + ienc = to_intel_encoder(connector_state-
> > > > > >
> > > > > >
> > > > > > best_encoder);
> > > > > + type = ienc->type;
> > > > > + }
> > > > > +
> > > > > if (connector_state->crtc != &crtc->base)
> > > > > continue;
> > > > >
> > > > > - connected_sink_compute_bpp(to_intel_connector(connect
> > > > > or),
> > > > > - pipe_config);
> > > > > + /* For DP compliance we need to ensure that we can
> > > > > override
> > > > > + * the computed bpp for the pipe.
> > > > > + */
> > > > > + if (type == INTEL_OUTPUT_DISPLAYPORT) {
> > > > > + struct intel_dp *intel_dp =
> > > > > + enc_to_intel_dp(connector_state-
> > > > > >
> > > > > >
> > > > > > best_encoder);
> > > > > +
> > > > > + if (intel_dp &&
> > > > > + (intel_dp->compliance_force_bpc != 0)) {
> > > > > + pipe_config->pipe_bpp =
> > > > > + intel_dp-
> > > > > >compliance_force_bpc*3;
> > > > > + DRM_DEBUG_KMS("JMB Setting pipe_bpp
> > > > > to
> > > > > %d\n",
> > > > > + pipe_config->pipe_bpp);
> > > > > + } else {
> > > > > + connected_sink_compute_bpp(to_intel_c
> > > > > onne
> > > > > ctor
> > > > > (connector),
> > > > > + pipe_config);
> > > > This kind of thing should be done in the encoder compute_config hook.
> > > Even though it's really not specific to an individual encoder
> > > configuration?
> > > This seems to be the central place where we're ensuring that we have a
> > > sane
> > > value for bpp relative to the display, and thus a good place to set this
> > > override to make compliance happy (which needs a specific bpc value for
> > > some
> > > test cases rather than one that is deemed sane relative to the sink's
> > > capabilities.
> > Well, this code path is only reached when the DisplayPort associated with a
> > given encoder is in the middle of compliance testing. I'd say that's very
> > encoder specific.
> >
> > The bpp computation happens in two phases. Here a baseline is computed,
> > considering what is generally supported by the hardware. The encoders are
> > allowed to override that value. You can look at HDMI for an example: it may
> > require a bpp override since HDMI doesn't supports 10 bpc, only 8 or 12. You
> > can
> > find similar code also in LVDS and even DP.
> >
> > Unfortunately, there is very little documentation of what the hooks are
> > supposed
> > to do. But for the question at hand, yes, it should really be in
> > intel_dp_compute_config().
> >
> > Ander
> Inside of intel_dp_compute_config(), do we need to change the pipe_config-
> >pipe_bpp
> to use intel_dp->compliance_force_bpc in case of CTS or should we just modify
> the local
> bpp value to use the force_bpc?
I think is enough to change the bpp variable before the loop that decides the
link rate and lane count, similarly to what is done in the eDP case when a bpp
value is read form the VBT.
Ander
>
> Manasi
> >
> >
> > >
> > >
> > > >
> > > >
> > > >
> > > > >
> > > > >
> > > > > + }
> > > > > + } else {
> > > > > + connected_sink_compute_bpp(to_intel_connector
> > > > > (con
> > > > > nect
> > > > > or),
> > > > > + pipe_config);
> > > > > + }
> > > > > }
> > > > >
> > > > > return bpp;
> > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > > index e23eed7..10eaff8 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > > @@ -865,6 +865,7 @@ struct intel_dp {
> > > > > unsigned long compliance_test_type;
> > > > > unsigned long compliance_test_data;
> > > > > bool compliance_test_active;
> > > > > + unsigned long compliance_force_bpc; /* 0 for default or bpc
> > > > > to
> > > > > use */
> > > > There's no code for setting compliance_test_active anywhere. The commit
> > > > message
> > > > mentions debugfs, but I didn't see anything related in the patch.
> > > It appears that Manasi forgot to include one of the patches I had sent
> > > her.
> > > The debugfs support code was in a separate patch.
> > >
> > > Jim
> > >
> > > >
> > > >
> > > >
> > > >
> > > > Ander
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list