From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com ([192.55.52.88]:25744 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932724AbeGFQcm (ORCPT ); Fri, 6 Jul 2018 12:32:42 -0400 Date: Fri, 6 Jul 2018 19:32:35 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Russell King - ARM Linux Cc: Dmitry Osipenko , Maarten Lankhorst , Laurent Pinchart , Thierry Reding , Neil Armstrong , Maxime Ripard , dri-devel@lists.freedesktop.org, Paul Kocialkowski , Thomas Hellstrom , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Ben Skeggs , Rodrigo Vivi , linux-tegra@vger.kernel.org, linux-media@vger.kernel.org Subject: Re: [RFC PATCH v3 1/2] drm: Add generic colorkey properties for DRM planes Message-ID: <20180706163235.GO5565@intel.com> References: <20180603220059.17670-1-digetx@gmail.com> <20180706141010.GJ5565@intel.com> <2295190.xHWjP7Ltc3@dimapc> <20180706154027.GB17271@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180706154027.GB17271@n2100.armlinux.org.uk> Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: On Fri, Jul 06, 2018 at 04:40:27PM +0100, Russell King - ARM Linux wrote: > On Fri, Jul 06, 2018 at 05:58:50PM +0300, Dmitry Osipenko wrote: > > On Friday, 6 July 2018 17:10:10 MSK Ville Syrj�l� wrote: > > > IIRC my earlier idea was to have different colorkey modes for the > > > min+max and value+mask modes. That way userspace might actually have > > > some chance of figuring out which bits of state actually do something. > > > Although for Intel hw I think the general rule is that min+max for YUV, > > > value+mask for RGB, so it's still not 100% clear what to pick if the > > > plane supports both. > > > > > > I guess one alternative would be to have min+max only, and the driver > > > would reject 'min != max' if it only uses a single value? > > > > > > > You should pick both and reject unsupported property values based on the > > planes framebuffer format. So it will be possible to set unsupported values > > while plane is disabled because it doesn't have an associated framebuffer and > > then atomic check will fail to enable plane if property values are invalid for > > the given format. > > The colorkey which is attached to a plane 'A' is not applied to plane > 'A', so the format of plane 'A' is not relevant. The colorkey is > applied to some other plane which will be below this plane in terms > of the plane blending operation. > > What if you have several planes below plane 'A' with differing > framebuffer formats - maybe an ARGB8888 plane and a ARGB1555 plane - > do you decide to limit the colorkey to 8bits per channel, or to > ARGB1555 format? > > The answer is, of course, hardware dependent - generic code can't > know the details of the colorkey implementation, which could be one > of: > > lower plane data -> expand to 8bpc -> match ARGB8888 colorkey > lower plane data -> match ARGB8888 reduced to plane compatible colorkey > > which will give different results depending on the format of the > lower plane data. Yeah. This is one of the other issues I highlighted previously. On some Intel hw you enable the dst colorkey on the lower plane where you paint the colorkey, on others you enable it on the plane above that gets shown/hidden based on the key match on the lower plane. I think on most of our hardware dst keying only ever happens between two planes, even if more planes are enabled on the crtc. But there is at least one exception where you can have two overlay planes that get keyed against the same primary plane. I was questioning which one is the better model for the uapi. Either way you still have the uncertainty of which planes actually participate in the keying process. One way around that would be to expose some kind of plane mask which would indicate the set of planes taking part in the keying process. I think for that to work we would have to move to a model where you enable the dst key on the lower plane where you paint the color key, otherwise having multiple bits set in the mask wouldn't make sense. That would perhaps also make it more clear what the format of the color key will be. So as an example that exception Intel hw case would have: overlay 2: key_mode={src,none} overlay 1: key_mode={src,none} primary: key_mode={dst,none}, dst_key_plane_mask=0x6 to indicate that both overlay planes participate in the keying process. Whereas most other Intel hw would have: overlay 2: key_mode={src,none} overlay 1: key_mode={src,none} primary: key_mode={dst,none}, dst_key_plane_mask=0x2 to indicate that only the overlay immediately above the primary will participate. -- Ville Syrj�l� Intel From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [RFC PATCH v3 1/2] drm: Add generic colorkey properties for DRM planes Date: Fri, 6 Jul 2018 19:32:35 +0300 Message-ID: <20180706163235.GO5565@intel.com> References: <20180603220059.17670-1-digetx@gmail.com> <20180706141010.GJ5565@intel.com> <2295190.xHWjP7Ltc3@dimapc> <20180706154027.GB17271@n2100.armlinux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20180706154027.GB17271@n2100.armlinux.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Russell King - ARM Linux Cc: linux-renesas-soc@vger.kernel.org, Thomas Hellstrom , Laurent Pinchart , Neil Armstrong , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Paul Kocialkowski , Thierry Reding , Ben Skeggs , Rodrigo Vivi , linux-tegra@vger.kernel.org, Dmitry Osipenko , Maxime Ripard , linux-media@vger.kernel.org List-Id: linux-tegra@vger.kernel.org T24gRnJpLCBKdWwgMDYsIDIwMTggYXQgMDQ6NDA6MjdQTSArMDEwMCwgUnVzc2VsbCBLaW5nIC0g QVJNIExpbnV4IHdyb3RlOgo+IE9uIEZyaSwgSnVsIDA2LCAyMDE4IGF0IDA1OjU4OjUwUE0gKzAz MDAsIERtaXRyeSBPc2lwZW5rbyB3cm90ZToKPiA+IE9uIEZyaWRheSwgNiBKdWx5IDIwMTggMTc6 MTA6MTAgTVNLIFZpbGxlIFN5cmrDpGzDpCB3cm90ZToKPiA+ID4gSUlSQyBteSBlYXJsaWVyIGlk ZWEgd2FzIHRvIGhhdmUgZGlmZmVyZW50IGNvbG9ya2V5IG1vZGVzIGZvciB0aGUKPiA+ID4gbWlu K21heCBhbmQgdmFsdWUrbWFzayBtb2Rlcy4gVGhhdCB3YXkgdXNlcnNwYWNlIG1pZ2h0IGFjdHVh bGx5IGhhdmUKPiA+ID4gc29tZSBjaGFuY2Ugb2YgZmlndXJpbmcgb3V0IHdoaWNoIGJpdHMgb2Yg c3RhdGUgYWN0dWFsbHkgZG8gc29tZXRoaW5nLgo+ID4gPiBBbHRob3VnaCBmb3IgSW50ZWwgaHcg SSB0aGluayB0aGUgZ2VuZXJhbCBydWxlIGlzIHRoYXQgbWluK21heCBmb3IgWVVWLAo+ID4gPiB2 YWx1ZSttYXNrIGZvciBSR0IsIHNvIGl0J3Mgc3RpbGwgbm90IDEwMCUgY2xlYXIgd2hhdCB0byBw aWNrIGlmIHRoZQo+ID4gPiBwbGFuZSBzdXBwb3J0cyBib3RoLgo+ID4gPiAKPiA+ID4gSSBndWVz cyBvbmUgYWx0ZXJuYXRpdmUgd291bGQgYmUgdG8gaGF2ZSBtaW4rbWF4IG9ubHksIGFuZCB0aGUg ZHJpdmVyCj4gPiA+IHdvdWxkIHJlamVjdCAnbWluICE9IG1heCcgaWYgaXQgb25seSB1c2VzIGEg c2luZ2xlIHZhbHVlPwo+ID4gPiAKPiA+IAo+ID4gWW91IHNob3VsZCBwaWNrIGJvdGggYW5kIHJl amVjdCB1bnN1cHBvcnRlZCBwcm9wZXJ0eSB2YWx1ZXMgYmFzZWQgb24gdGhlIAo+ID4gcGxhbmVz IGZyYW1lYnVmZmVyIGZvcm1hdC4gU28gaXQgd2lsbCBiZSBwb3NzaWJsZSB0byBzZXQgdW5zdXBw b3J0ZWQgdmFsdWVzIAo+ID4gd2hpbGUgcGxhbmUgaXMgZGlzYWJsZWQgYmVjYXVzZSBpdCBkb2Vz bid0IGhhdmUgYW4gYXNzb2NpYXRlZCBmcmFtZWJ1ZmZlciBhbmQgCj4gPiB0aGVuIGF0b21pYyBj aGVjayB3aWxsIGZhaWwgdG8gZW5hYmxlIHBsYW5lIGlmIHByb3BlcnR5IHZhbHVlcyBhcmUgaW52 YWxpZCBmb3IgCj4gPiB0aGUgZ2l2ZW4gZm9ybWF0Lgo+IAo+IFRoZSBjb2xvcmtleSB3aGljaCBp cyBhdHRhY2hlZCB0byBhIHBsYW5lICdBJyBpcyBub3QgYXBwbGllZCB0byBwbGFuZQo+ICdBJywg c28gdGhlIGZvcm1hdCBvZiBwbGFuZSAnQScgaXMgbm90IHJlbGV2YW50LiAgVGhlIGNvbG9ya2V5 IGlzCj4gYXBwbGllZCB0byBzb21lIG90aGVyIHBsYW5lIHdoaWNoIHdpbGwgYmUgYmVsb3cgdGhp cyBwbGFuZSBpbiB0ZXJtcwo+IG9mIHRoZSBwbGFuZSBibGVuZGluZyBvcGVyYXRpb24uCj4gCj4g V2hhdCBpZiB5b3UgaGF2ZSBzZXZlcmFsIHBsYW5lcyBiZWxvdyBwbGFuZSAnQScgd2l0aCBkaWZm ZXJpbmcKPiBmcmFtZWJ1ZmZlciBmb3JtYXRzIC0gbWF5YmUgYW4gQVJHQjg4ODggcGxhbmUgYW5k IGEgQVJHQjE1NTUgcGxhbmUgLQo+IGRvIHlvdSBkZWNpZGUgdG8gbGltaXQgdGhlIGNvbG9ya2V5 IHRvIDhiaXRzIHBlciBjaGFubmVsLCBvciB0bwo+IEFSR0IxNTU1IGZvcm1hdD8KPiAKPiBUaGUg YW5zd2VyIGlzLCBvZiBjb3Vyc2UsIGhhcmR3YXJlIGRlcGVuZGVudCAtIGdlbmVyaWMgY29kZSBj YW4ndAo+IGtub3cgdGhlIGRldGFpbHMgb2YgdGhlIGNvbG9ya2V5IGltcGxlbWVudGF0aW9uLCB3 aGljaCBjb3VsZCBiZSBvbmUKPiBvZjoKPiAKPiAgIGxvd2VyIHBsYW5lIGRhdGEgLT4gZXhwYW5k IHRvIDhicGMgLT4gbWF0Y2ggQVJHQjg4ODggY29sb3JrZXkKPiAgIGxvd2VyIHBsYW5lIGRhdGEg LT4gbWF0Y2ggQVJHQjg4ODggcmVkdWNlZCB0byBwbGFuZSBjb21wYXRpYmxlIGNvbG9ya2V5Cj4g Cj4gd2hpY2ggd2lsbCBnaXZlIGRpZmZlcmVudCByZXN1bHRzIGRlcGVuZGluZyBvbiB0aGUgZm9y bWF0IG9mIHRoZQo+IGxvd2VyIHBsYW5lIGRhdGEuCgpZZWFoLiBUaGlzIGlzIG9uZSBvZiB0aGUg b3RoZXIgaXNzdWVzIEkgaGlnaGxpZ2h0ZWQgcHJldmlvdXNseS4gT24gc29tZQpJbnRlbCBodyB5 b3UgZW5hYmxlIHRoZSBkc3QgY29sb3JrZXkgb24gdGhlIGxvd2VyIHBsYW5lIHdoZXJlIHlvdSBw YWludAp0aGUgY29sb3JrZXksIG9uIG90aGVycyB5b3UgZW5hYmxlIGl0IG9uIHRoZSBwbGFuZSBh Ym92ZSB0aGF0IGdldHMKc2hvd24vaGlkZGVuIGJhc2VkIG9uIHRoZSBrZXkgbWF0Y2ggb24gdGhl IGxvd2VyIHBsYW5lLiBJIHRoaW5rIG9uIG1vc3QKb2Ygb3VyIGhhcmR3YXJlIGRzdCBrZXlpbmcg b25seSBldmVyIGhhcHBlbnMgYmV0d2VlbiB0d28gcGxhbmVzLCBldmVuCmlmIG1vcmUgcGxhbmVz IGFyZSBlbmFibGVkIG9uIHRoZSBjcnRjLiBCdXQgdGhlcmUgaXMgYXQgbGVhc3Qgb25lCmV4Y2Vw dGlvbiB3aGVyZSB5b3UgY2FuIGhhdmUgdHdvIG92ZXJsYXkgcGxhbmVzIHRoYXQgZ2V0IGtleWVk IGFnYWluc3QKdGhlIHNhbWUgcHJpbWFyeSBwbGFuZS4KCkkgd2FzIHF1ZXN0aW9uaW5nIHdoaWNo IG9uZSBpcyB0aGUgYmV0dGVyIG1vZGVsIGZvciB0aGUgdWFwaS4gRWl0aGVyCndheSB5b3Ugc3Rp bGwgaGF2ZSB0aGUgdW5jZXJ0YWludHkgb2Ygd2hpY2ggcGxhbmVzIGFjdHVhbGx5IHBhcnRpY2lw YXRlCmluIHRoZSBrZXlpbmcgcHJvY2Vzcy4gT25lIHdheSBhcm91bmQgdGhhdCB3b3VsZCBiZSB0 byBleHBvc2Ugc29tZSBraW5kCm9mIHBsYW5lIG1hc2sgd2hpY2ggd291bGQgaW5kaWNhdGUgdGhl IHNldCBvZiBwbGFuZXMgdGFraW5nIHBhcnQgaW4gdGhlCmtleWluZyBwcm9jZXNzLiBJIHRoaW5r IGZvciB0aGF0IHRvIHdvcmsgd2Ugd291bGQgaGF2ZSB0byBtb3ZlIHRvIGEKbW9kZWwgd2hlcmUg eW91IGVuYWJsZSB0aGUgZHN0IGtleSBvbiB0aGUgbG93ZXIgcGxhbmUgd2hlcmUgeW91IHBhaW50 CnRoZSBjb2xvciBrZXksIG90aGVyd2lzZSBoYXZpbmcgbXVsdGlwbGUgYml0cyBzZXQgaW4gdGhl IG1hc2sgd291bGRuJ3QKbWFrZSBzZW5zZS4gVGhhdCB3b3VsZCBwZXJoYXBzIGFsc28gbWFrZSBp dCBtb3JlIGNsZWFyIHdoYXQgdGhlCmZvcm1hdCBvZiB0aGUgY29sb3Iga2V5IHdpbGwgYmUuCgpT byBhcyBhbiBleGFtcGxlIHRoYXQgZXhjZXB0aW9uIEludGVsIGh3IGNhc2Ugd291bGQgaGF2ZToK IG92ZXJsYXkgMjoga2V5X21vZGU9e3NyYyxub25lfQogb3ZlcmxheSAxOiBrZXlfbW9kZT17c3Jj LG5vbmV9CiBwcmltYXJ5OiAgIGtleV9tb2RlPXtkc3Qsbm9uZX0sIGRzdF9rZXlfcGxhbmVfbWFz az0weDYKCnRvIGluZGljYXRlIHRoYXQgYm90aCBvdmVybGF5IHBsYW5lcyBwYXJ0aWNpcGF0ZSBp biB0aGUga2V5aW5nCnByb2Nlc3MuCgpXaGVyZWFzIG1vc3Qgb3RoZXIgSW50ZWwgaHcgd291bGQg aGF2ZToKIG92ZXJsYXkgMjoga2V5X21vZGU9e3NyYyxub25lfQogb3ZlcmxheSAxOiBrZXlfbW9k ZT17c3JjLG5vbmV9CiBwcmltYXJ5OiAgIGtleV9tb2RlPXtkc3Qsbm9uZX0sIGRzdF9rZXlfcGxh bmVfbWFzaz0weDIKCnRvIGluZGljYXRlIHRoYXQgb25seSB0aGUgb3ZlcmxheSBpbW1lZGlhdGVs eSBhYm92ZSB0aGUgcHJpbWFyeQp3aWxsIHBhcnRpY2lwYXRlLgoKLS0gClZpbGxlIFN5cmrDpGzD pApJbnRlbApfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpk cmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0 cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mga01.intel.com ([192.55.52.88]:25744 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932724AbeGFQcm (ORCPT ); Fri, 6 Jul 2018 12:32:42 -0400 Date: Fri, 6 Jul 2018 19:32:35 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Russell King - ARM Linux Cc: Dmitry Osipenko , Maarten Lankhorst , Laurent Pinchart , Thierry Reding , Neil Armstrong , Maxime Ripard , dri-devel@lists.freedesktop.org, Paul Kocialkowski , Thomas Hellstrom , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Ben Skeggs , Rodrigo Vivi , linux-tegra@vger.kernel.org, linux-media@vger.kernel.org Subject: Re: [RFC PATCH v3 1/2] drm: Add generic colorkey properties for DRM planes Message-ID: <20180706163235.GO5565@intel.com> References: <20180603220059.17670-1-digetx@gmail.com> <20180706141010.GJ5565@intel.com> <2295190.xHWjP7Ltc3@dimapc> <20180706154027.GB17271@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180706154027.GB17271@n2100.armlinux.org.uk> Sender: linux-media-owner@vger.kernel.org List-ID: On Fri, Jul 06, 2018 at 04:40:27PM +0100, Russell King - ARM Linux wrote: > On Fri, Jul 06, 2018 at 05:58:50PM +0300, Dmitry Osipenko wrote: > > On Friday, 6 July 2018 17:10:10 MSK Ville Syrjälä wrote: > > > IIRC my earlier idea was to have different colorkey modes for the > > > min+max and value+mask modes. That way userspace might actually have > > > some chance of figuring out which bits of state actually do something. > > > Although for Intel hw I think the general rule is that min+max for YUV, > > > value+mask for RGB, so it's still not 100% clear what to pick if the > > > plane supports both. > > > > > > I guess one alternative would be to have min+max only, and the driver > > > would reject 'min != max' if it only uses a single value? > > > > > > > You should pick both and reject unsupported property values based on the > > planes framebuffer format. So it will be possible to set unsupported values > > while plane is disabled because it doesn't have an associated framebuffer and > > then atomic check will fail to enable plane if property values are invalid for > > the given format. > > The colorkey which is attached to a plane 'A' is not applied to plane > 'A', so the format of plane 'A' is not relevant. The colorkey is > applied to some other plane which will be below this plane in terms > of the plane blending operation. > > What if you have several planes below plane 'A' with differing > framebuffer formats - maybe an ARGB8888 plane and a ARGB1555 plane - > do you decide to limit the colorkey to 8bits per channel, or to > ARGB1555 format? > > The answer is, of course, hardware dependent - generic code can't > know the details of the colorkey implementation, which could be one > of: > > lower plane data -> expand to 8bpc -> match ARGB8888 colorkey > lower plane data -> match ARGB8888 reduced to plane compatible colorkey > > which will give different results depending on the format of the > lower plane data. Yeah. This is one of the other issues I highlighted previously. On some Intel hw you enable the dst colorkey on the lower plane where you paint the colorkey, on others you enable it on the plane above that gets shown/hidden based on the key match on the lower plane. I think on most of our hardware dst keying only ever happens between two planes, even if more planes are enabled on the crtc. But there is at least one exception where you can have two overlay planes that get keyed against the same primary plane. I was questioning which one is the better model for the uapi. Either way you still have the uncertainty of which planes actually participate in the keying process. One way around that would be to expose some kind of plane mask which would indicate the set of planes taking part in the keying process. I think for that to work we would have to move to a model where you enable the dst key on the lower plane where you paint the color key, otherwise having multiple bits set in the mask wouldn't make sense. That would perhaps also make it more clear what the format of the color key will be. So as an example that exception Intel hw case would have: overlay 2: key_mode={src,none} overlay 1: key_mode={src,none} primary: key_mode={dst,none}, dst_key_plane_mask=0x6 to indicate that both overlay planes participate in the keying process. Whereas most other Intel hw would have: overlay 2: key_mode={src,none} overlay 1: key_mode={src,none} primary: key_mode={dst,none}, dst_key_plane_mask=0x2 to indicate that only the overlay immediately above the primary will participate. -- Ville Syrjälä Intel