| Message ID | 1439546611-3191-5-git-send-email-mika.kahola@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 14, 2015 at 01:03:24PM +0300, Mika Kahola wrote: > It is possible the we request to have a mode that has > higher pixel clock than our HW can support. This patch > checks if requested pixel clock is lower than the one > supported by the HW. The requested mode is discarded > if we cannot support the requested pixel clock. > > This patch applies to LVDS. > > V2: > - removed computation for max pixel clock > > V3: > - cleanup by removing unnecessary lines > > V4: > - moved supported dotclock check from mode_valid() to intel_lvds_init() > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > --- > drivers/gpu/drm/i915/intel_lvds.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 881b5d1..06b9d1b 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -1098,8 +1098,11 @@ void intel_lvds_init(struct drm_device *dev) > drm_mode_debug_printmodeline(scan); > > fixed_mode = drm_mode_duplicate(dev, scan); > - if (fixed_mode) > - goto out; > + if (fixed_mode) { > + if (fixed_mode->clock <= dev_priv->max_dotclk_freq) > + goto out; > + } > + fixed_mode = NULL; > } > } > > @@ -1111,8 +1114,10 @@ void intel_lvds_init(struct drm_device *dev) > fixed_mode = drm_mode_duplicate(dev, dev_priv->vbt.lfp_lvds_vbt_mode); > if (fixed_mode) { > fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; > - goto out; > + if (fixed_mode->clock <= dev_priv->max_dotclk_freq) > + goto out; > } > + fixed_mode = NULL; > } > > /* > @@ -1135,8 +1140,10 @@ void intel_lvds_init(struct drm_device *dev) > DRM_DEBUG_KMS("using current (BIOS) mode: "); > drm_mode_debug_printmodeline(fixed_mode); > fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; > - goto out; > + if (fixed_mode->clock <= dev_priv->max_dotclk_freq) > + goto out; > } > + fixed_mode = NULL; I don't think we want to special case just LVDS this way. Whatever we do with fixed_mode validation should be done for all connector types that have it. For now I think we can more or less ignore the issue. So in that light, your previous patch was OK except it should have just checked fixed_mode->clock instead of mode->clock. Sorry if my ramblings confused you too much. > } > > /* If we still don't have a mode after all that, give up. */ > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 2015-08-14 at 16:09 +0300, Ville Syrjälä wrote: > On Fri, Aug 14, 2015 at 01:03:24PM +0300, Mika Kahola wrote: > > It is possible the we request to have a mode that has > > higher pixel clock than our HW can support. This patch > > checks if requested pixel clock is lower than the one > > supported by the HW. The requested mode is discarded > > if we cannot support the requested pixel clock. > > > > This patch applies to LVDS. > > > > V2: > > - removed computation for max pixel clock > > > > V3: > > - cleanup by removing unnecessary lines > > > > V4: > > - moved supported dotclock check from mode_valid() to intel_lvds_init() > > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > > --- > > drivers/gpu/drm/i915/intel_lvds.c | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > > index 881b5d1..06b9d1b 100644 > > --- a/drivers/gpu/drm/i915/intel_lvds.c > > +++ b/drivers/gpu/drm/i915/intel_lvds.c > > @@ -1098,8 +1098,11 @@ void intel_lvds_init(struct drm_device *dev) > > drm_mode_debug_printmodeline(scan); > > > > fixed_mode = drm_mode_duplicate(dev, scan); > > - if (fixed_mode) > > - goto out; > > + if (fixed_mode) { > > + if (fixed_mode->clock <= dev_priv->max_dotclk_freq) > > + goto out; > > + } > > + fixed_mode = NULL; > > } > > } > > > > @@ -1111,8 +1114,10 @@ void intel_lvds_init(struct drm_device *dev) > > fixed_mode = drm_mode_duplicate(dev, dev_priv->vbt.lfp_lvds_vbt_mode); > > if (fixed_mode) { > > fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; > > - goto out; > > + if (fixed_mode->clock <= dev_priv->max_dotclk_freq) > > + goto out; > > } > > + fixed_mode = NULL; > > } > > > > /* > > @@ -1135,8 +1140,10 @@ void intel_lvds_init(struct drm_device *dev) > > DRM_DEBUG_KMS("using current (BIOS) mode: "); > > drm_mode_debug_printmodeline(fixed_mode); > > fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; > > - goto out; > > + if (fixed_mode->clock <= dev_priv->max_dotclk_freq) > > + goto out; > > } > > + fixed_mode = NULL; > > I don't think we want to special case just LVDS this way. Whatever we do > with fixed_mode validation should be done for all connector types that > have it. For now I think we can more or less ignore the issue. > > So in that light, your previous patch was OK except it should have just > checked fixed_mode->clock instead of mode->clock. Sorry if my ramblings > confused you too much. > No worries. I was a bit unsure if this check should have been done in init anyway. I'll take a step back and modify the previous patch. -Mika- > > } > > > > /* If we still don't have a mode after all that, give up. */ > > -- > > 1.9.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
Hi Mika, On Fri, Aug 14, 2015 at 01:03:24PM +0300, Mika Kahola wrote: > It is possible the we request to have a mode that has > higher pixel clock than our HW can support. This patch > checks if requested pixel clock is lower than the one > supported by the HW. The requested mode is discarded > if we cannot support the requested pixel clock. > > This patch applies to LVDS. The spec lists maximum dot clocks for each connector type. Here's the spec for IVB for instance (see section 2.3): https://01.org/sites/default/files/documentation/ivb_ihd_os_vol3_part4.pdf E.g. for LVDS it's 224 MHz, for DP it's 270 MHz. If this is identical across all generations (which I haven't checked, but seems likely), you can use these constant values, which would be more accurate than just using the same dev_priv->max_cdclk_freq for all connector types. Best regards, Lukas
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 881b5d1..06b9d1b 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -1098,8 +1098,11 @@ void intel_lvds_init(struct drm_device *dev) drm_mode_debug_printmodeline(scan); fixed_mode = drm_mode_duplicate(dev, scan); - if (fixed_mode) - goto out; + if (fixed_mode) { + if (fixed_mode->clock <= dev_priv->max_dotclk_freq) + goto out; + } + fixed_mode = NULL; } } @@ -1111,8 +1114,10 @@ void intel_lvds_init(struct drm_device *dev) fixed_mode = drm_mode_duplicate(dev, dev_priv->vbt.lfp_lvds_vbt_mode); if (fixed_mode) { fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; - goto out; + if (fixed_mode->clock <= dev_priv->max_dotclk_freq) + goto out; } + fixed_mode = NULL; } /* @@ -1135,8 +1140,10 @@ void intel_lvds_init(struct drm_device *dev) DRM_DEBUG_KMS("using current (BIOS) mode: "); drm_mode_debug_printmodeline(fixed_mode); fixed_mode->type |= DRM_MODE_TYPE_PREFERRED; - goto out; + if (fixed_mode->clock <= dev_priv->max_dotclk_freq) + goto out; } + fixed_mode = NULL; } /* If we still don't have a mode after all that, give up. */
It is possible the we request to have a mode that has higher pixel clock than our HW can support. This patch checks if requested pixel clock is lower than the one supported by the HW. The requested mode is discarded if we cannot support the requested pixel clock. This patch applies to LVDS. V2: - removed computation for max pixel clock V3: - cleanup by removing unnecessary lines V4: - moved supported dotclock check from mode_valid() to intel_lvds_init() Signed-off-by: Mika Kahola <mika.kahola@intel.com> --- drivers/gpu/drm/i915/intel_lvds.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)