On Fri, 2015-12-04 at 19:04 +0200, Ville Syrjälä wrote: > On Fri, Dec 04, 2015 at 05:51:56PM +0100, Daniel Vetter wrote: > > On Fri, Dec 04, 2015 at 04:15:27PM +0000, Vivi, Rodrigo wrote: > > > On Fri, 2015-12-04 at 16:05 +0100, Daniel Vetter wrote: > > > > On Wed, Dec 02, 2015 at 02:30:28PM +0200, Jani Nikula wrote: > > > > > On Wed, 02 Dec 2015, Ville Syrjälä < > > > > > ville.syrjala@xxxxxxxxxxxxxxx> > > > > > wrote: > > > > > > On Tue, Dec 01, 2015 at 05:11:40PM -0800, Wayne Boyer > > > > > > wrote: > > > > > > > The cherryview device shares many characteristics with > > > > > > > the > > > > > > > valleyview > > > > > > > device. When support was added to the driver for > > > > > > > cherryview, > > > > > > > the > > > > > > > corresponding device info structure included > > > > > > > .is_valleyview = > > > > > > > 1. > > > > > > > This is not correct and leads to some confusion. > > > > > > > > > > > > > > This patch changes .is_valleyview to .is_cherryview in > > > > > > > the > > > > > > > cherryview > > > > > > > device info structure and defines the > > > > > > > HAS_GEN7_LP_FEATURES > > > > > > > macro. > > > > > > > Then where appropriate, instances of IS_VALLEYVIEW are > > > > > > > replaced > > > > > > > with > > > > > > > HAS_GEN7_LP_FEATURES to test for either a valleyview or a > > > > > > > cherryview > > > > > > > device. > > > > > > > > > > > > I don't like the name of the macro. Most of the shared bits > > > > > > are > > > > > > display > > > > > > related, so we could have something like HAS_VLV_DISPLAY. > > > > > > For the > > > > > > rest, > > > > > > I think we could just test IS_VLV||IS_CHV explicitly. > > > > > > Unless > > > > > > someone > > > > > > can come up with a better name that covers everything? > > > > > > > > > > Definitely NAK on HAS_GEN7_LP_FEATURES. > > > > > > > > > > HAS_VLV_DISPLAY would be a subset of HAS_GMCH_DISPLAY, which > > > > > I > > > > > guess > > > > > wouldn't be that bad... unless someone starts using that for > > > > > a > > > > > VLV||CHV > > > > > shorthand in non-display code. > > > > > > > > > > I think I might just go for the verbose (IS_VALLEYVIEW || > > > > > IS_CHERRYVIEW) > > > > > all around. Especially since we've been brainwashed with the > > > > > old > > > > > vlv is > > > > > both vlv and chv code. > > > > > > > > HAS_GMCH_DISPLAY is what I've generally used, since usually you > > > > have > > > > a > > > > INTEL_INFO()->gen >= 5 test already somewhere. If we want to > > > > make > > > > this > > > > more explicit the proper name for vlv is BAYTRAIL, and for > > > > truely byt > > > > specific stuff we've named things byt_. So what about Defining > > > > an > > > > IS_BAYTRAIL instead for the cases where it's not vlv || chv. > > > > > > Baytrail is the platform name with the Valleyview graphics. Than > > > we > > > would have Cherry Trail and/or Braswell for Cherryview graphics > > > and > > > Apollo Lake for Broxton. So I would vote to stay with Valleyview, > > > Cherryview and Broxton only. > > > > > > > > > > > And what's the benefit of all this churn? > > > > > > Organize and prepare the code for future platforms. > > > Avoid more confusion like we had on IS_SKYLAKE x IS_KABYLAKE. > > > Make things more easy and clear if we decide to add .is_atom_lp > > > on > > > these platforms definition. > > > > .is_atom_lp is imo the more sensible change to do, since it > > includes bxt. > > BXT vs. VLV/CHV have practically nothing in common in the driver, > so I wouldn't go there. this was exactly the point where HAS_GEN7_LP_FEATURES appeared, than for BXT+ would be HAS_GEN9_LP_FEATURES otherwises with .is_atom_lp for gen7 it would be ((gen == 7 || gen ==8) && .is_atom_lp); and for gen 9: (gen ==9 && .is_atom_lp) IS_GEN9_LP seems a good name, but IS_GEN7_LP doesn't so it was related to the FEATURES macros at platform definition... But yeah, maybe better just stay with full version (is_vlv || is_chv) for that and if makes sense for future platforms we add the .is_atom_lp later... > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
