Re: [PATCH 3/4] drm/i915: CSC color correction

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Sep 10, 2014 at 03:17:34PM -0700, Matt Roper wrote:
> On Tue, Sep 09, 2014 at 11:53:15AM +0530, shashank.sharma@intel.com wrote:
> > From: Shashank Sharma <shashank.sharma@intel.com>
> > 
> > This patch adds support for CSC correction color property.
> > It does the following:
> > 1. Creates a new DRM property for CSC correction. Adds this into
> >    mode_config.
> ...
> 
> One thing I forgot to ask in my response yesterday...does it make sense
> to create this property in dev->mode_config rather than in dev_priv?
> Some properties (like the plane rotation property that went in a little
> while ago) do seem like a good match for dev->mode_config because
> they're generic concepts that a lot of different drivers will want to
> reuse (and in some cases the common core/helper library code might want
> to make use of them as well, as we do in restore_fbdev_mode()).  But
> it's unclear to me whether non-Intel hardware handles color correction
> similarly enough that the CSC property you're creating here is something
> other drivers will ever use, or whether it will always be an
> Intel-specific thing.  My understanding is that the values here are very
> Intel-specific, so there's never going to be an opportunity for
> core/helper library code to make use of the property, right?  If this
> does turn out to be an Intel-only property, then it's probably better to
> move it to dev_priv (where we have a couple of our connector properties
> today).
> 
> If you do decide that there's potential for reuse by other drivers and
> that you want to keep this in dev->mode_config you'll want to make sure
> you Cc the dri-devel mailing list on the patches that touch the DRM core
> files.

The csc matrix looks platform-specific enough (I expect it'll be different
on big-core, but haven't checked tbh) that a generic property imo doesn't
make sense. But others from the color manager task certainly qualify,
which is another reason not to smash them all together into a tightly knit
boundle.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch