diff mbox

[v8,10/12] drm/i915: Update bits per component for display info

Message ID 1471430989-28161-12-git-send-email-mika.kahola@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kahola Aug. 17, 2016, 10:49 a.m. UTC
DisplayPort branch device may define max supported bits per
component. Update display info based on this value if bpc
is defined.

v2: cleanup to match the drm_dp_helper.c patches introduced
    earlier in this series
v3: Fill bpc for connector's display info in separate
    drm_dp_helper function (Daniel)
v4: remove updating bpc for display info as it may be overridden
    when parsing EDID. Instead, check bpc for DP branch device
    during compute_config

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

jim.bride@linux.intel.com Sept. 7, 2016, 9:27 p.m. UTC | #1
On Wed, Aug 17, 2016 at 01:49:47PM +0300, Mika Kahola wrote:
> DisplayPort branch device may define max supported bits per
> component. Update display info based on this value if bpc
> is defined.
> 
> v2: cleanup to match the drm_dp_helper.c patches introduced
>     earlier in this series
> v3: Fill bpc for connector's display info in separate
>     drm_dp_helper function (Daniel)
> v4: remove updating bpc for display info as it may be overridden
>     when parsing EDID. Instead, check bpc for DP branch device
>     during compute_config
> 
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 25f459e..17110d1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1524,6 +1524,20 @@ void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>  	}
>  }
>  
> +int intel_dp_compute_bpp(struct intel_dp *intel_dp,
> +			 struct intel_crtc_state *pipe_config)

Indentation seems off.

> +{
> +	int bpp, bpc;
> +
> +	bpp = pipe_config->pipe_bpp;
> +	bpc = drm_dp_downstream_max_bpc(intel_dp->dpcd, intel_dp->downstream_ports);
> +
> +	if (bpc > 0)

Do we need to ensure that bpp is sane before this calculation as well?

> +		bpp = min(bpp, 3*bpc);
> +
> +	return bpp;
> +}
> +
>  bool
>  intel_dp_compute_config(struct intel_encoder *encoder,
>  			struct intel_crtc_state *pipe_config)

Indentation again.

Jim

> @@ -1589,7 +1603,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  
>  	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
>  	 * bpc in between. */
> -	bpp = pipe_config->pipe_bpp;
> +	bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
> +
>  	if (is_edp(intel_dp)) {
>  
>  		/* Get bpp from vbt only for panels that dont have bpp in edid */
> -- 
> 1.9.1
Mika Kahola Sept. 8, 2016, 12:30 p.m. UTC | #2
Thanks for the review. I'll fix those indentations.

> -----Original Message-----
> From: Jim Bride [mailto:jim.bride@linux.intel.com]
> Sent: Thursday, September 8, 2016 12:27 AM
> To: Kahola, Mika <mika.kahola@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> ville.syrjala@linux.intel.com; daniel.vetter@ffwll.ch
> Subject: Re: [PATCH v8 10/12] drm/i915: Update bits per component for
> display info
> 
> On Wed, Aug 17, 2016 at 01:49:47PM +0300, Mika Kahola wrote:
> > DisplayPort branch device may define max supported bits per component.
> > Update display info based on this value if bpc is defined.
> >
> > v2: cleanup to match the drm_dp_helper.c patches introduced
> >     earlier in this series
> > v3: Fill bpc for connector's display info in separate
> >     drm_dp_helper function (Daniel)
> > v4: remove updating bpc for display info as it may be overridden
> >     when parsing EDID. Instead, check bpc for DP branch device
> >     during compute_config
> >
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index 25f459e..17110d1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1524,6 +1524,20 @@ void intel_dp_compute_rate(struct intel_dp
> *intel_dp, int port_clock,
> >  	}
> >  }
> >
> > +int intel_dp_compute_bpp(struct intel_dp *intel_dp,
> > +			 struct intel_crtc_state *pipe_config)
> 
> Indentation seems off.
> 
> > +{
> > +	int bpp, bpc;
> > +
> > +	bpp = pipe_config->pipe_bpp;
> > +	bpc = drm_dp_downstream_max_bpc(intel_dp->dpcd,
> > +intel_dp->downstream_ports);
> > +
> > +	if (bpc > 0)
> 
> Do we need to ensure that bpp is sane before this calculation as well?
drm_dp_downstream_max_bpc() routine returns 0 if we can't find bpc from DPCD. Therefore bpc sanity is checked so to ensure that bpp has some meaningful value other than 0.

> 
> > +		bpp = min(bpp, 3*bpc);
> > +
> > +	return bpp;
> > +}
> > +
> >  bool
> >  intel_dp_compute_config(struct intel_encoder *encoder,
> >  			struct intel_crtc_state *pipe_config)
> 
> Indentation again.
> 
> Jim
> 
> > @@ -1589,7 +1603,8 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> >
> >  	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
> >  	 * bpc in between. */
> > -	bpp = pipe_config->pipe_bpp;
> > +	bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
> > +
> >  	if (is_edp(intel_dp)) {
> >
> >  		/* Get bpp from vbt only for panels that dont have bpp in
> edid */
> > --
> > 1.9.1
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 25f459e..17110d1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1524,6 +1524,20 @@  void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
 	}
 }
 
+int intel_dp_compute_bpp(struct intel_dp *intel_dp,
+			 struct intel_crtc_state *pipe_config)
+{
+	int bpp, bpc;
+
+	bpp = pipe_config->pipe_bpp;
+	bpc = drm_dp_downstream_max_bpc(intel_dp->dpcd, intel_dp->downstream_ports);
+
+	if (bpc > 0)
+		bpp = min(bpp, 3*bpc);
+
+	return bpp;
+}
+
 bool
 intel_dp_compute_config(struct intel_encoder *encoder,
 			struct intel_crtc_state *pipe_config)
@@ -1589,7 +1603,8 @@  intel_dp_compute_config(struct intel_encoder *encoder,
 
 	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
 	 * bpc in between. */
-	bpp = pipe_config->pipe_bpp;
+	bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
+
 	if (is_edp(intel_dp)) {
 
 		/* Get bpp from vbt only for panels that dont have bpp in edid */