| Message ID | 1363198868-21787-4-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 13, 2013 at 11:21:02AM -0700, Ben Widawsky wrote: > Given certain fusing options discussed in the previous patch, it's > possible to end up with platforms that normally have PCH but that PCH > doesn't actually exist. In many cases, this is easily remedied with > setting 0 pipes. This covers the other corners. > > Requiring this is a symptom of improper code splitting (using > HAS_PCH_SPLIT instead of proper GEN checking, basically). I do not want > to fix this. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_display.c | 4 +++- > drivers/gpu/drm/i915/intel_lvds.c | 4 ++++ > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 587dca0..ceed199 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -461,6 +461,7 @@ enum intel_pch { > PCH_IBX, /* Ibexpeak PCH */ > PCH_CPT, /* Cougarpoint PCH */ > PCH_LPT, /* Lynxpoint PCH */ > + PCH_NOP, > }; > > enum intel_sbi_destination { > @@ -1351,6 +1352,7 @@ struct drm_i915_file_private { > #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT) > #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) > #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) > +#define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) > #define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE) > > #define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)->has_force_wake) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index d6dbffd..1f0624e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5085,7 +5085,9 @@ static void lpt_init_pch_refclk(struct drm_device *dev) > */ > void intel_init_pch_refclk(struct drm_device *dev) > { Since this function here only deals with display refclocks I think a num_pipes == 0 check should equally work. And also makes it clear what's exactly going on. > - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) > + if (HAS_PCH_NOP(dev)) > + return; > + else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) > ironlake_init_pch_refclk(dev); > else if (HAS_PCH_LPT(dev)) > lpt_init_pch_refclk(dev); > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 6b24fc5..613ac43 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -1018,6 +1018,10 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder) > > static bool intel_lvds_supported(struct drm_device *dev) > { > + > + if (HAS_PCH_NOP(dev)) > + return false; This hunk here looks superflous, imo we should stop any and all output probing much earlier in the callchain. -Daniel > + > /* With the introduction of the PCH we gained a dedicated > * LVDS presence pin, use it. */ > if (HAS_PCH_SPLIT(dev)) > -- > 1.8.1.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Sun, Mar 17, 2013 at 10:05:41PM +0100, Daniel Vetter wrote: > On Wed, Mar 13, 2013 at 11:21:02AM -0700, Ben Widawsky wrote: > > Given certain fusing options discussed in the previous patch, it's > > possible to end up with platforms that normally have PCH but that PCH > > doesn't actually exist. In many cases, this is easily remedied with > > setting 0 pipes. This covers the other corners. > > > > Requiring this is a symptom of improper code splitting (using > > HAS_PCH_SPLIT instead of proper GEN checking, basically). I do not want > > to fix this. > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_display.c | 4 +++- > > drivers/gpu/drm/i915/intel_lvds.c | 4 ++++ > > 3 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 587dca0..ceed199 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -461,6 +461,7 @@ enum intel_pch { > > PCH_IBX, /* Ibexpeak PCH */ > > PCH_CPT, /* Cougarpoint PCH */ > > PCH_LPT, /* Lynxpoint PCH */ > > + PCH_NOP, > > }; > > > > enum intel_sbi_destination { > > @@ -1351,6 +1352,7 @@ struct drm_i915_file_private { > > #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT) > > #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) > > #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) > > +#define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) > > #define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE) > > > > #define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)->has_force_wake) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index d6dbffd..1f0624e 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5085,7 +5085,9 @@ static void lpt_init_pch_refclk(struct drm_device *dev) > > */ > > void intel_init_pch_refclk(struct drm_device *dev) > > { > > Since this function here only deals with display refclocks I think a > num_pipes == 0 check should equally work. And also makes it clear what's > exactly going on. See my comment on the first patch, I think if we never run the code in setup_outputs we should be fine here already and don't need anything more. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 587dca0..ceed199 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -461,6 +461,7 @@ enum intel_pch { PCH_IBX, /* Ibexpeak PCH */ PCH_CPT, /* Cougarpoint PCH */ PCH_LPT, /* Lynxpoint PCH */ + PCH_NOP, }; enum intel_sbi_destination { @@ -1351,6 +1352,7 @@ struct drm_i915_file_private { #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT) #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) +#define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) #define HAS_PCH_SPLIT(dev) (INTEL_PCH_TYPE(dev) != PCH_NONE) #define HAS_FORCE_WAKE(dev) (INTEL_INFO(dev)->has_force_wake) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d6dbffd..1f0624e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5085,7 +5085,9 @@ static void lpt_init_pch_refclk(struct drm_device *dev) */ void intel_init_pch_refclk(struct drm_device *dev) { - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) + if (HAS_PCH_NOP(dev)) + return; + else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) ironlake_init_pch_refclk(dev); else if (HAS_PCH_LPT(dev)) lpt_init_pch_refclk(dev); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 6b24fc5..613ac43 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -1018,6 +1018,10 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder) static bool intel_lvds_supported(struct drm_device *dev) { + + if (HAS_PCH_NOP(dev)) + return false; + /* With the introduction of the PCH we gained a dedicated * LVDS presence pin, use it. */ if (HAS_PCH_SPLIT(dev))
Given certain fusing options discussed in the previous patch, it's possible to end up with platforms that normally have PCH but that PCH doesn't actually exist. In many cases, this is easily remedied with setting 0 pipes. This covers the other corners. Requiring this is a symptom of improper code splitting (using HAS_PCH_SPLIT instead of proper GEN checking, basically). I do not want to fix this. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_display.c | 4 +++- drivers/gpu/drm/i915/intel_lvds.c | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-)