From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/4] drm/i915: CSC color correction Date: Thu, 11 Sep 2014 09:53:50 +0200 Message-ID: <20140911075350.GC15520@phenom.ffwll.local> References: <20140726015853.GA8878@intel.com> <1410243796-11172-1-git-send-email-shashank.sharma@intel.com> <1410243796-11172-4-git-send-email-shashank.sharma@intel.com> <20140910221734.GI26681@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f175.google.com (mail-we0-f175.google.com [74.125.82.175]) by gabe.freedesktop.org (Postfix) with ESMTP id 5404589A86 for ; Thu, 11 Sep 2014 00:53:25 -0700 (PDT) Received: by mail-we0-f175.google.com with SMTP id w61so4832626wes.20 for ; Thu, 11 Sep 2014 00:53:24 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140910221734.GI26681@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Matt Roper Cc: daniel.vetter@intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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 > > > > 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