Re: [Intel-gfx] [PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle

[Ville Syrjälä]

On Thu, Aug 11, 2016 at 03:26:42PM +0300, Mika Kahola wrote:
> On Thu, 2016-08-11 at 12:56 +0200, Daniel Vetter wrote:
> > On Thu, Aug 11, 2016 at 01:51:42PM +0300, Ville Syrjälä wrote:
> > > On Thu, Aug 11, 2016 at 12:43:39PM +0300, Mika Kahola wrote:
> > > > On Thu, 2016-08-11 at 10:18 +0300, Ville Syrjälä wrote:
> > > > > On Mon, Aug 08, 2016 at 04:00:28PM +0300, Mika Kahola wrote:
> > > > > > Filter out a mode that exceeds the max pixel rate setting
> > > > > > for DP to VGA dongle. This is defined in DPCD register 0x81
> > > > > > if detailed cap info i.e. info field is 4 bytes long and
> > > > > > it is available for DP downstream port.
> > > > > > 
> > > > > > The register defines the pixel rate divided by 8 in MP/s.
> > > > > > 
> > > > > > v2: DPCD read outs and computation moved to drm (Ville, Daniel)
> > > > > > v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock()
> > > > > > function (Daniel)
> > > > > > v4: Use of drm_dp_helper.c routines to compute max pixel clock 
> > > > > > (Ville)
> > > > > > v5: Use of intel_dp->downstream_ports to read out port capabilities.
> > > > > > Code restructuring (Ville)
> > > > > > v6: Move DP branch device check to drm_dp_helper.c (Daniel)
> > > > > > 
> > > > > > Signed-off-by: Mika Kahola 
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_dp.c | 25 -
> > > > > >  1 file changed, 24 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > index 21b04c3..e990c8b 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > @@ -190,6 +190,26 @@ intel_dp_max_data_rate(int max_link_clock, int 
> > > > > > max_lanes)
> > > > > > return (max_link_clock * max_lanes * 8) / 10;
> > > > > >  }
> > > > > >  
> > > > > > +static int
> > > > > > +intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp, int 
> > > > > > dotclk)
> > > > > > +{
> > > > > 
> > > > > I would just
> > > > > 
> > > > > {
> > > > >   int max_dotclk = dev_priv->max_dotclk_freq;
> > > > > 
> > > > >   ds_max_dotclk = ...;
> > > > >   if (ds_dotclk != 0)
> > > > >   max_dotclk = min(max_dotclk, ds_max_dotclk);
> > > > > 
> > > > >   return max_dotclk;
> > > > > }
> > > > > 
> > > > > > +   int ds_dotclk;
> > > > > > +   int type;
> > > > > > +
> > > > > > +   ds_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> > > > > > +   
> > > > > > intel_dp->downstream_ports);
> > > > > > +
> > > > > > +   if (ds_dotclk == 0)
> > > > > > +   return dotclk;
> > > > > > +
> > > > > > +   type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> > > > > > +   
> > > > > > +   if (type != DP_DS_PORT_TYPE_VGA)
> > > > > > +   return dotclk;
> > > > > 
> > > > > Why isn't drm_dp_downstream_max_clock() handling all of it already?
> > > > > Why are we even checking for !=VGA?
> > > > The routine drm_dp_downstream_max_clock returns the clock rate which can
> > > > be dotclock (VGA only) or TMDS clock (for DVI, HDMI, DP++). Here, we
> > > > need to have a check for this as we are only interested to update VGA
> > > > dotclock value.
> > > 
> > > We should handle it all. Actually I'm not even sure how we're supposed
> > > to deal with the downstream port max TMDS clock since for HDMI that
> > > depends on the bpc, but since this is about a DP->HDMI conversion, I
> > > don't know if we have to take the downstream port max TMDS clock into
> > > account when choosing the bpc over the DP link as well. I suppose that's
> > > possible if the dongle can't change change the bpc, and instead just
> > > passes things through. I think this is one of those places where the
> > > DP spec is way too unclear. But for DP->VGA there is no clock going out
> > > the other end, so it must be just about the limits of the DP input or
> > > the DAC.
> > 
> > I guess we should defensively assume that the tmds clock limit is both for
> > the input and the output signal, worst case, for the dp->hdmi dongle?
> > Except when it's a passive level-shifter only one ofc.
> > -Daniel
> So, we should respect dongles both tmds and bpc values. I thought that
> tmds clock checks was already covered by Ville's DP dual mode patch
> series?

That doesn't deal with active DP->HDMI converters, which is what this
DPCD stuff will have to deal with.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle

[Mika Kahola]

On Thu, 2016-08-11 at 12:56 +0200, Daniel Vetter wrote:
> On Thu, Aug 11, 2016 at 01:51:42PM +0300, Ville Syrjälä wrote:
> > On Thu, Aug 11, 2016 at 12:43:39PM +0300, Mika Kahola wrote:
> > > On Thu, 2016-08-11 at 10:18 +0300, Ville Syrjälä wrote:
> > > > On Mon, Aug 08, 2016 at 04:00:28PM +0300, Mika Kahola wrote:
> > > > > Filter out a mode that exceeds the max pixel rate setting
> > > > > for DP to VGA dongle. This is defined in DPCD register 0x81
> > > > > if detailed cap info i.e. info field is 4 bytes long and
> > > > > it is available for DP downstream port.
> > > > > 
> > > > > The register defines the pixel rate divided by 8 in MP/s.
> > > > > 
> > > > > v2: DPCD read outs and computation moved to drm (Ville, Daniel)
> > > > > v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock()
> > > > > function (Daniel)
> > > > > v4: Use of drm_dp_helper.c routines to compute max pixel clock (Ville)
> > > > > v5: Use of intel_dp->downstream_ports to read out port capabilities.
> > > > > Code restructuring (Ville)
> > > > > v6: Move DP branch device check to drm_dp_helper.c (Daniel)
> > > > > 
> > > > > Signed-off-by: Mika Kahola 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_dp.c | 25 -
> > > > >  1 file changed, 24 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index 21b04c3..e990c8b 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -190,6 +190,26 @@ intel_dp_max_data_rate(int max_link_clock, int 
> > > > > max_lanes)
> > > > >   return (max_link_clock * max_lanes * 8) / 10;
> > > > >  }
> > > > >  
> > > > > +static int
> > > > > +intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp, int 
> > > > > dotclk)
> > > > > +{
> > > > 
> > > > I would just
> > > > 
> > > > {
> > > > int max_dotclk = dev_priv->max_dotclk_freq;
> > > > 
> > > > ds_max_dotclk = ...;
> > > > if (ds_dotclk != 0)
> > > > max_dotclk = min(max_dotclk, ds_max_dotclk);
> > > > 
> > > > return max_dotclk;
> > > > }
> > > > 
> > > > > + int ds_dotclk;
> > > > > + int type;
> > > > > +
> > > > > + ds_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> > > > > + 
> > > > > intel_dp->downstream_ports);
> > > > > +
> > > > > + if (ds_dotclk == 0)
> > > > > + return dotclk;
> > > > > +
> > > > > + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> > > > > + 
> > > > > + if (type != DP_DS_PORT_TYPE_VGA)
> > > > > + return dotclk;
> > > > 
> > > > Why isn't drm_dp_downstream_max_clock() handling all of it already?
> > > > Why are we even checking for !=VGA?
> > > The routine drm_dp_downstream_max_clock returns the clock rate which can
> > > be dotclock (VGA only) or TMDS clock (for DVI, HDMI, DP++). Here, we
> > > need to have a check for this as we are only interested to update VGA
> > > dotclock value.
> > 
> > We should handle it all. Actually I'm not even sure how we're supposed
> > to deal with the downstream port max TMDS clock since for HDMI that
> > depends on the bpc, but since this is about a DP->HDMI conversion, I
> > don't know if we have to take the downstream port max TMDS clock into
> > account when choosing the bpc over the DP link as well. I suppose that's
> > possible if the dongle can't change change the bpc, and instead just
> > passes things through. I think this is one of those places where the
> > DP spec is way too unclear. But for DP->VGA there is no clock going out
> > the other end, so it must be just about the limits of the DP input or
> > the DAC.
> 
> I guess we should defensively assume that the tmds clock limit is both for
> the input and the output signal, worst case, for the dp->hdmi dongle?
> Except when it's a passive level-shifter only one ofc.
> -Daniel
So, we should respect dongles both tmds and bpc values. I thought that
tmds clock checks was already covered by Ville's DP dual mode patch
series?


-- 
Mika Kahola - Intel OTC

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle

[Daniel Vetter]

On Thu, Aug 11, 2016 at 01:51:42PM +0300, Ville Syrjälä wrote:
> On Thu, Aug 11, 2016 at 12:43:39PM +0300, Mika Kahola wrote:
> > On Thu, 2016-08-11 at 10:18 +0300, Ville Syrjälä wrote:
> > > On Mon, Aug 08, 2016 at 04:00:28PM +0300, Mika Kahola wrote:
> > > > Filter out a mode that exceeds the max pixel rate setting
> > > > for DP to VGA dongle. This is defined in DPCD register 0x81
> > > > if detailed cap info i.e. info field is 4 bytes long and
> > > > it is available for DP downstream port.
> > > > 
> > > > The register defines the pixel rate divided by 8 in MP/s.
> > > > 
> > > > v2: DPCD read outs and computation moved to drm (Ville, Daniel)
> > > > v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock()
> > > > function (Daniel)
> > > > v4: Use of drm_dp_helper.c routines to compute max pixel clock (Ville)
> > > > v5: Use of intel_dp->downstream_ports to read out port capabilities.
> > > > Code restructuring (Ville)
> > > > v6: Move DP branch device check to drm_dp_helper.c (Daniel)
> > > > 
> > > > Signed-off-by: Mika Kahola 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 25 -
> > > >  1 file changed, 24 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 21b04c3..e990c8b 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -190,6 +190,26 @@ intel_dp_max_data_rate(int max_link_clock, int 
> > > > max_lanes)
> > > > return (max_link_clock * max_lanes * 8) / 10;
> > > >  }
> > > >  
> > > > +static int
> > > > +intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp, int dotclk)
> > > > +{
> > > 
> > > I would just
> > > 
> > > {
> > >   int max_dotclk = dev_priv->max_dotclk_freq;
> > > 
> > >   ds_max_dotclk = ...;
> > >   if (ds_dotclk != 0)
> > >   max_dotclk = min(max_dotclk, ds_max_dotclk);
> > > 
> > >   return max_dotclk;
> > > }
> > > 
> > > > +   int ds_dotclk;
> > > > +   int type;
> > > > +
> > > > +   ds_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> > > > +   
> > > > intel_dp->downstream_ports);
> > > > +
> > > > +   if (ds_dotclk == 0)
> > > > +   return dotclk;
> > > > +
> > > > +   type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> > > > +   
> > > > +   if (type != DP_DS_PORT_TYPE_VGA)
> > > > +   return dotclk;
> > > 
> > > Why isn't drm_dp_downstream_max_clock() handling all of it already?
> > > Why are we even checking for !=VGA?
> > The routine drm_dp_downstream_max_clock returns the clock rate which can
> > be dotclock (VGA only) or TMDS clock (for DVI, HDMI, DP++). Here, we
> > need to have a check for this as we are only interested to update VGA
> > dotclock value.
> 
> We should handle it all. Actually I'm not even sure how we're supposed
> to deal with the downstream port max TMDS clock since for HDMI that
> depends on the bpc, but since this is about a DP->HDMI conversion, I
> don't know if we have to take the downstream port max TMDS clock into
> account when choosing the bpc over the DP link as well. I suppose that's
> possible if the dongle can't change change the bpc, and instead just
> passes things through. I think this is one of those places where the
> DP spec is way too unclear. But for DP->VGA there is no clock going out
> the other end, so it must be just about the limits of the DP input or
> the DAC.

I guess we should defensively assume that the tmds clock limit is both for
the input and the output signal, worst case, for the dp->hdmi dongle?
Except when it's a passive level-shifter only one ofc.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle

[Ville Syrjälä]

On Thu, Aug 11, 2016 at 12:43:39PM +0300, Mika Kahola wrote:
> On Thu, 2016-08-11 at 10:18 +0300, Ville Syrjälä wrote:
> > On Mon, Aug 08, 2016 at 04:00:28PM +0300, Mika Kahola wrote:
> > > Filter out a mode that exceeds the max pixel rate setting
> > > for DP to VGA dongle. This is defined in DPCD register 0x81
> > > if detailed cap info i.e. info field is 4 bytes long and
> > > it is available for DP downstream port.
> > > 
> > > The register defines the pixel rate divided by 8 in MP/s.
> > > 
> > > v2: DPCD read outs and computation moved to drm (Ville, Daniel)
> > > v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock()
> > > function (Daniel)
> > > v4: Use of drm_dp_helper.c routines to compute max pixel clock (Ville)
> > > v5: Use of intel_dp->downstream_ports to read out port capabilities.
> > > Code restructuring (Ville)
> > > v6: Move DP branch device check to drm_dp_helper.c (Daniel)
> > > 
> > > Signed-off-by: Mika Kahola 
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 25 -
> > >  1 file changed, 24 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 21b04c3..e990c8b 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -190,6 +190,26 @@ intel_dp_max_data_rate(int max_link_clock, int 
> > > max_lanes)
> > >   return (max_link_clock * max_lanes * 8) / 10;
> > >  }
> > >  
> > > +static int
> > > +intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp, int dotclk)
> > > +{
> > 
> > I would just
> > 
> > {
> > int max_dotclk = dev_priv->max_dotclk_freq;
> > 
> > ds_max_dotclk = ...;
> > if (ds_dotclk != 0)
> > max_dotclk = min(max_dotclk, ds_max_dotclk);
> > 
> > return max_dotclk;
> > }
> > 
> > > + int ds_dotclk;
> > > + int type;
> > > +
> > > + ds_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> > > + intel_dp->downstream_ports);
> > > +
> > > + if (ds_dotclk == 0)
> > > + return dotclk;
> > > +
> > > + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> > > + 
> > > + if (type != DP_DS_PORT_TYPE_VGA)
> > > + return dotclk;
> > 
> > Why isn't drm_dp_downstream_max_clock() handling all of it already?
> > Why are we even checking for !=VGA?
> The routine drm_dp_downstream_max_clock returns the clock rate which can
> be dotclock (VGA only) or TMDS clock (for DVI, HDMI, DP++). Here, we
> need to have a check for this as we are only interested to update VGA
> dotclock value.

We should handle it all. Actually I'm not even sure how we're supposed
to deal with the downstream port max TMDS clock since for HDMI that
depends on the bpc, but since this is about a DP->HDMI conversion, I
don't know if we have to take the downstream port max TMDS clock into
account when choosing the bpc over the DP link as well. I suppose that's
possible if the dongle can't change change the bpc, and instead just
passes things through. I think this is one of those places where the
DP spec is way too unclear. But for DP->VGA there is no clock going out
the other end, so it must be just about the limits of the DP input or
the DAC.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle

[Mika Kahola]

On Thu, 2016-08-11 at 10:18 +0300, Ville Syrjälä wrote:
> On Mon, Aug 08, 2016 at 04:00:28PM +0300, Mika Kahola wrote:
> > Filter out a mode that exceeds the max pixel rate setting
> > for DP to VGA dongle. This is defined in DPCD register 0x81
> > if detailed cap info i.e. info field is 4 bytes long and
> > it is available for DP downstream port.
> > 
> > The register defines the pixel rate divided by 8 in MP/s.
> > 
> > v2: DPCD read outs and computation moved to drm (Ville, Daniel)
> > v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock()
> > function (Daniel)
> > v4: Use of drm_dp_helper.c routines to compute max pixel clock (Ville)
> > v5: Use of intel_dp->downstream_ports to read out port capabilities.
> > Code restructuring (Ville)
> > v6: Move DP branch device check to drm_dp_helper.c (Daniel)
> > 
> > Signed-off-by: Mika Kahola 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 25 -
> >  1 file changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 21b04c3..e990c8b 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -190,6 +190,26 @@ intel_dp_max_data_rate(int max_link_clock, int 
> > max_lanes)
> > return (max_link_clock * max_lanes * 8) / 10;
> >  }
> >  
> > +static int
> > +intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp, int dotclk)
> > +{
> 
> I would just
> 
> {
>   int max_dotclk = dev_priv->max_dotclk_freq;
> 
>   ds_max_dotclk = ...;
>   if (ds_dotclk != 0)
>   max_dotclk = min(max_dotclk, ds_max_dotclk);
> 
>   return max_dotclk;
> }
> 
> > +   int ds_dotclk;
> > +   int type;
> > +
> > +   ds_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> > +   intel_dp->downstream_ports);
> > +
> > +   if (ds_dotclk == 0)
> > +   return dotclk;
> > +
> > +   type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> > +   
> > +   if (type != DP_DS_PORT_TYPE_VGA)
> > +   return dotclk;
> 
> Why isn't drm_dp_downstream_max_clock() handling all of it already?
> Why are we even checking for !=VGA?
The routine drm_dp_downstream_max_clock returns the clock rate which can
be dotclock (VGA only) or TMDS clock (for DVI, HDMI, DP++). Here, we
need to have a check for this as we are only interested to update VGA
dotclock value.

Cheers,
Mika
> 
> > +
> > +   return min(dotclk, ds_dotclk);
> > +}
> > +
> >  static enum drm_mode_status
> >  intel_dp_mode_valid(struct drm_connector *connector,
> > struct drm_display_mode *mode)
> > @@ -199,7 +219,10 @@ intel_dp_mode_valid(struct drm_connector *connector,
> > struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> > int target_clock = mode->clock;
> > int max_rate, mode_rate, max_lanes, max_link_clock;
> > -   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> > +   int max_dotclk;
> > +
> > +   max_dotclk = intel_dp_downstream_max_dotclock(intel_dp,
> > + 
> > to_i915(connector->dev)->max_dotclk_freq);
> >  
> > if (is_edp(intel_dp) && fixed_mode) {
> > if (mode->hdisplay > fixed_mode->hdisplay)
> > -- 
> > 1.9.1
> 


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle

[Ville Syrjälä]

On Mon, Aug 08, 2016 at 04:00:28PM +0300, Mika Kahola wrote:
> Filter out a mode that exceeds the max pixel rate setting
> for DP to VGA dongle. This is defined in DPCD register 0x81
> if detailed cap info i.e. info field is 4 bytes long and
> it is available for DP downstream port.
> 
> The register defines the pixel rate divided by 8 in MP/s.
> 
> v2: DPCD read outs and computation moved to drm (Ville, Daniel)
> v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock()
> function (Daniel)
> v4: Use of drm_dp_helper.c routines to compute max pixel clock (Ville)
> v5: Use of intel_dp->downstream_ports to read out port capabilities.
> Code restructuring (Ville)
> v6: Move DP branch device check to drm_dp_helper.c (Daniel)
> 
> Signed-off-by: Mika Kahola 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 25 -
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 21b04c3..e990c8b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -190,6 +190,26 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
>   return (max_link_clock * max_lanes * 8) / 10;
>  }
>  
> +static int
> +intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp, int dotclk)
> +{

I would just

{
int max_dotclk = dev_priv->max_dotclk_freq;

ds_max_dotclk = ...;
if (ds_dotclk != 0)
max_dotclk = min(max_dotclk, ds_max_dotclk);

return max_dotclk;
}

> + int ds_dotclk;
> + int type;
> +
> + ds_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> + intel_dp->downstream_ports);
> +
> + if (ds_dotclk == 0)
> + return dotclk;
> +
> + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> + 
> + if (type != DP_DS_PORT_TYPE_VGA)
> + return dotclk;

Why isn't drm_dp_downstream_max_clock() handling all of it already?
Why are we even checking for !=VGA?

> +
> + return min(dotclk, ds_dotclk);
> +}
> +
>  static enum drm_mode_status
>  intel_dp_mode_valid(struct drm_connector *connector,
>   struct drm_display_mode *mode)
> @@ -199,7 +219,10 @@ intel_dp_mode_valid(struct drm_connector *connector,
>   struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
>   int target_clock = mode->clock;
>   int max_rate, mode_rate, max_lanes, max_link_clock;
> - int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> + int max_dotclk;
> +
> + max_dotclk = intel_dp_downstream_max_dotclock(intel_dp,
> +   
> to_i915(connector->dev)->max_dotclk_freq);
>  
>   if (is_edp(intel_dp) && fixed_mode) {
>   if (mode->hdisplay > fixed_mode->hdisplay)
> -- 
> 1.9.1

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v7 3/5] drm/i915: Check pixel rate for DP to VGA dongle

[Mika Kahola]

Filter out a mode that exceeds the max pixel rate setting
for DP to VGA dongle. This is defined in DPCD register 0x81
if detailed cap info i.e. info field is 4 bytes long and
it is available for DP downstream port.

The register defines the pixel rate divided by 8 in MP/s.

v2: DPCD read outs and computation moved to drm (Ville, Daniel)
v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock()
function (Daniel)
v4: Use of drm_dp_helper.c routines to compute max pixel clock (Ville)
v5: Use of intel_dp->downstream_ports to read out port capabilities.
Code restructuring (Ville)
v6: Move DP branch device check to drm_dp_helper.c (Daniel)

Signed-off-by: Mika Kahola 
---
 drivers/gpu/drm/i915/intel_dp.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 21b04c3..e990c8b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -190,6 +190,26 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
return (max_link_clock * max_lanes * 8) / 10;
 }
 
+static int
+intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp, int dotclk)
+{
+   int ds_dotclk;
+   int type;
+
+   ds_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
+   intel_dp->downstream_ports);
+
+   if (ds_dotclk == 0)
+   return dotclk;
+
+   type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
+
+   if (type != DP_DS_PORT_TYPE_VGA)
+   return dotclk;
+
+   return min(dotclk, ds_dotclk);
+}
+
 static enum drm_mode_status
 intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
@@ -199,7 +219,10 @@ intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
int target_clock = mode->clock;
int max_rate, mode_rate, max_lanes, max_link_clock;
-   int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
+   int max_dotclk;
+
+   max_dotclk = intel_dp_downstream_max_dotclock(intel_dp,
+ 
to_i915(connector->dev)->max_dotclk_freq);
 
if (is_edp(intel_dp) && fixed_mode) {
if (mode->hdisplay > fixed_mode->hdisplay)
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx