On Thu, Dec 08, 2016 at 11:11:50AM +0200, Jani Nikula wrote:
> On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@xxxxxxxxx> wrote:
> > On Thu, 08 Dec 2016, Jani Nikula <jani.nikula@xxxxxxxxx> wrote:
> >> This lets us use IS_MOBILE() for distinguishing the desktop and mobile
> >> parts instead of duplicating PCI IDs in Pineview specific checks.
> >
> > This probably needs more thorough review of the !IS_MOBILE() paths in
> > parts not touched by this patch.
>
> And Daniel says,
>
> 11:06 danvet j4ni, pnv is_mobile, even for the desktop version
> 11:06 danvet iirc
> 11:07 danvet because is_mobile has nothing to do with whether the
> chip is for laptops or not
>
> so better just forget about these two patches I guess.
Yeah, IS_MOBILE is seriously confusing in our code. On gen2/3 it was
essentially matching HAS_LVDS (but we never made that one), later on the
splits started to happened on different axis (atom vs big core and stuff
like that), but somehow IS_MOBILE stuck around to this day and is
sometimes abused to restrict w/a to specific chips. If someone wants to
clean this up:
- Remove IS_MOBILE on gen5+, use specific platforms/pci ids where we need
it.
- Rename IS_MOBILE to HAS_LVDS on gen2/3/4 (not entirely sure about gen4,
would need to double-check that it makes sense).
- Nuke is_mobile from the chip info, it's only misleading people. Mobile
is the future everywhere anyway ;-)
Cheers, Daniel
>
>
> BR,
> Jani.
>
>
> >
> > For example this one in i915_gem_detect_bit_6_swizzle() which was pretty
> > weird already before the Pineview device info patches:
> >
> > } else if (IS_MOBILE(dev_priv) ||
> > (IS_GEN3(dev_priv) &&
> > !IS_G33(dev_priv) && !IS_PINEVIEW(dev_priv))) {
> >
> > BR,
> > Jani.
> >
> >
> >>
> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> >> ---
> >> arch/x86/kernel/early-quirks.c | 3 ++-
> >> drivers/gpu/drm/i915/i915_drv.h | 2 --
> >> drivers/gpu/drm/i915/i915_pci.c | 12 ++++++++++--
> >> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> >> include/drm/i915_pciids.h | 6 ++++--
> >> 5 files changed, 18 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> >> index 6a08e25a48d8..34af418d88cc 100644
> >> --- a/arch/x86/kernel/early-quirks.c
> >> +++ b/arch/x86/kernel/early-quirks.c
> >> @@ -508,7 +508,8 @@ static const struct pci_device_id intel_early_ids[] __initconst = {
> >> INTEL_I945G_IDS(&gen3_early_ops),
> >> INTEL_I945GM_IDS(&gen3_early_ops),
> >> INTEL_VLV_IDS(&gen6_early_ops),
> >> - INTEL_PINEVIEW_IDS(&gen3_early_ops),
> >> + INTEL_PINEVIEW_D_IDS(&gen3_early_ops),
> >> + INTEL_PINEVIEW_M_IDS(&gen3_early_ops),
> >> INTEL_I965G_IDS(&gen3_early_ops),
> >> INTEL_G33_IDS(&gen3_early_ops),
> >> INTEL_I965GM_IDS(&gen3_early_ops),
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 1480e733312a..ee1726b28b82 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -2616,8 +2616,6 @@ intel_info(const struct drm_i915_private *dev_priv)
> >> #define IS_G45(dev_priv) ((dev_priv)->info.platform == INTEL_G45)
> >> #define IS_GM45(dev_priv) ((dev_priv)->info.platform == INTEL_GM45)
> >> #define IS_G4X(dev_priv) (IS_G45(dev_priv) || IS_GM45(dev_priv))
> >> -#define IS_PINEVIEW_G(dev_priv) (INTEL_DEVID(dev_priv) == 0xa001)
> >> -#define IS_PINEVIEW_M(dev_priv) (INTEL_DEVID(dev_priv) == 0xa011)
> >> #define IS_PINEVIEW(dev_priv) ((dev_priv)->info.platform == INTEL_PINEVIEW)
> >> #define IS_G33(dev_priv) ((dev_priv)->info.platform == INTEL_G33)
> >> #define IS_IRONLAKE_M(dev_priv) (INTEL_DEVID(dev_priv) == 0x0046)
> >> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> >> index 93f50ef2a309..451113f79499 100644
> >> --- a/drivers/gpu/drm/i915/i915_pci.c
> >> +++ b/drivers/gpu/drm/i915/i915_pci.c
> >> @@ -139,7 +139,14 @@ static const struct intel_device_info intel_g33_info = {
> >> .has_overlay = 1,
> >> };
> >>
> >> -static const struct intel_device_info intel_pineview_info = {
> >> +static const struct intel_device_info intel_pineview_d_info = {
> >> + GEN3_FEATURES,
> >> + .platform = INTEL_PINEVIEW,
> >> + .has_hotplug = 1,
> >> + .has_overlay = 1,
> >> +};
> >> +
> >> +static const struct intel_device_info intel_pineview_m_info = {
> >> GEN3_FEATURES,
> >> .platform = INTEL_PINEVIEW, .is_mobile = 1,
> >> .has_hotplug = 1,
> >> @@ -444,7 +451,8 @@ static const struct pci_device_id pciidlist[] = {
> >> INTEL_I965GM_IDS(&intel_i965gm_info),
> >> INTEL_GM45_IDS(&intel_gm45_info),
> >> INTEL_G45_IDS(&intel_g45_info),
> >> - INTEL_PINEVIEW_IDS(&intel_pineview_info),
> >> + INTEL_PINEVIEW_D_IDS(&intel_pineview_d_info),
> >> + INTEL_PINEVIEW_M_IDS(&intel_pineview_m_info),
> >> INTEL_IRONLAKE_D_IDS(&intel_ironlake_d_info),
> >> INTEL_IRONLAKE_M_IDS(&intel_ironlake_m_info),
> >> INTEL_SNB_D_IDS(&intel_sandybridge_d_info),
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 9171431558a3..afe07947e51c 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -659,7 +659,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc)
> >> u32 reg;
> >> unsigned long wm;
> >>
> >> - latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
> >> + latency = intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
> >> dev_priv->is_ddr3,
> >> dev_priv->fsb_freq,
> >> dev_priv->mem_freq);
> >> @@ -7730,7 +7730,7 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
> >> vlv_setup_wm_latency(dev_priv);
> >> dev_priv->display.update_wm = vlv_update_wm;
> >> } else if (IS_PINEVIEW(dev_priv)) {
> >> - if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
> >> + if (!intel_get_cxsr_latency(!IS_MOBILE(dev_priv),
> >> dev_priv->is_ddr3,
> >> dev_priv->fsb_freq,
> >> dev_priv->mem_freq)) {
> >> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> >> index 540be9ff0346..9c226eb1788f 100644
> >> --- a/include/drm/i915_pciids.h
> >> +++ b/include/drm/i915_pciids.h
> >> @@ -100,8 +100,10 @@
> >> INTEL_VGA_DEVICE(0x2e42, info), /* B43_G */ \
> >> INTEL_VGA_DEVICE(0x2e92, info) /* B43_G.1 */
> >>
> >> -#define INTEL_PINEVIEW_IDS(info) \
> >> - INTEL_VGA_DEVICE(0xa001, info), \
> >> +#define INTEL_PINEVIEW_D_IDS(info) \
> >> + INTEL_VGA_DEVICE(0xa001, info)
> >> +
> >> +#define INTEL_PINEVIEW_M_IDS(info) \
> >> INTEL_VGA_DEVICE(0xa011, info)
> >>
> >> #define INTEL_IRONLAKE_D_IDS(info) \
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
