Re: [PATCH v9] drm/i915/skl: Add support for SKL background color

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Apr 27, 2015 at 08:33:38PM +0000, Konduru, Chandra wrote:
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > Sent: Monday, April 27, 2015 8:28 AM
> > On Fri, Feb 20, 2015 at 04:11:33PM -0800, Chandra Konduru wrote:
> > > This patch adds support for Skylake display pipe background color.
> > > +	<td valign="top" >Background color in 16bpc BGR (B-MSB, R-LSB)</td>
> > 
> > I'd prefer to settle on ARGB msb->lsb layout, unless there's already precedent
> > for something else. I think it's the most common component order around, and
> > so should result in least amount of code to stuff the bits into the hardware
> > registers.
> 
> To give some background on this, initially using ARGB msb->lsb layout to stuff
> bit into hw regs, but changed to BGR msb->lsb per feedback. Before reversing/
> respunning, would like to have an agreement. Daniel/Damien, are you ok with 
> reversing the order to align with hw? Or just keep the way current patch doing?

Imo if we want to really end up with a real standard we should have a
macro to encode ARGB values and 4 macros to extract the individual parts
in some uapi header and use that instead of open-coding shifts. That would
need to be a drm core header ofc. The exact layout is probably irrelevant
anyway.
-Daniel

> 
> > 
> > > +static int intel_crtc_set_property(struct drm_crtc *crtc,
> > > +	struct drm_property *property, uint64_t val) {
> > > +	struct drm_device *dev = crtc->dev;
> > > +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > > +	int i;
> > > +
> > > +	if (INTEL_INFO(dev)->gen >= 9) {
> > > +		if (property == dev->mode_config.background_color_property)
> > {
> > > +			uint64_t bottom = 0;
> > > +
> > > +			/* BGR 16bpc ==> RGB 10bpc */
> > > +			for (i = 0; i < 3; i++)
> > > +				bottom |= (((val >> (i * 16 + 6)) & 0x3FF) << ((2 -
> > i) * 10));
> > > +
> > > +			intel_crtc->background_color = (uint32_t) bottom;
> > > +
> > > +			skl_crtc_set_background_color(intel_crtc);
> > > +		}
> > > +		return 0;
> > > +	}
> > > +	return -EINVAL;
> > > +}
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx