[Intel-gfx] [RFC, PATCH] drm/i915: split display functions by chip type
jbarnes at virtuousgeek.org
Tue Sep 15 17:46:00 CEST 2009
On Tue, 15 Sep 2009 15:22:28 +0800
Zhenyu Wang <zhenyuw at linux.intel.com> wrote:
> On 2009.09.14 16:38:08 -0700, Jesse Barnes wrote:
> > On Mon, 14 Sep 2009 15:29:46 -0700
> > Keith Packard <keithp at keithp.com> wrote:
> > > On Mon, 2009-09-14 at 14:51 -0700, Jesse Barnes wrote:
> > > > This patch is a long time coming IMO, and there are many more
> > > > cleanups possible. It splits out several of the display
> > > > functions into a separate display function table to avoid tons
> > > > of chipset specific if..else if..else if blocks all over.
> > > >
> > > > Zhenyu, what do you think? Should help with adding new hardware
> > > > support over time and make functionality a bit clearer.
> > >
> > > I worry that we'll end up with piles of nearly-duplicated code and
> > > forget to replicate bug fixes across all chips.
> > >
> > > Any idea how to balance this?
> > Yeah, we should avoid having a bunch of duplicated code. If that's
> > happening we haven't split things correctly, IMO.
> > In this set there's a little boilerplate for getting FIFO sizes
> > (just so we can do I915_READ etc) and the existing DPMS code
> > duplication (not improved or worsened with this patch).
> > Boilerplate like the FIFO stuff could be removed by changing the
> > args to the FIFO routines.
> > Or were you worried about something in particular? Maybe with the
> > suggestions I outlined in the structure definition?
> You did further work than I was thinking. I'd like to see something
> like intel_init_display() to initialize all features to some internal
> mask bits, and your function assignment work looks also fine with me
> in consider of current splitted function hooks.
Yeah there are probably cases where a simple bitfield of features would
be better than a full set of separate functions. I'm definitely not
opposed to adding that too...
Jesse Barnes, Intel Open Source Technology Center
More information about the Intel-gfx