From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Wed, 18 Jan 2017 22:49:41 +0200 Subject: [RFC/RFT PATCH 4/4] drm/bridge: dw-hdmi: Take input format from plat_data In-Reply-To: <66fc9acc-5db9-d124-fbe5-31893a5bd5df@baylibre.com> References: <1484656294-6140-1-git-send-email-narmstrong@baylibre.com> <66fc9acc-5db9-d124-fbe5-31893a5bd5df@baylibre.com> Message-ID: <3138084.ubGyLVHoXm@avalon> To: linus-amlogic@lists.infradead.org List-Id: linus-amlogic.lists.infradead.org Hi Neil, (CC'ing Hans Verkuil) On Wednesday 18 Jan 2017 12:24:22 Neil Armstrong wrote: > On 01/18/2017 11:28 AM, Jose Abreu wrote: > > On 17-01-2017 12:31, Neil Armstrong wrote: > >> Some display pipelines can only provide non-RBG input pixels to the HDMI > >> TX Controller, this patch takes the pixel format from the plat_data if > >> provided. > >> > >> Signed-off-by: Neil Armstrong > >> --- > >> > >> drivers/gpu/drm/bridge/dw-hdmi.c | 7 +++++-- > >> include/drm/bridge/dw_hdmi.h | 9 +++++++++ > >> 2 files changed, 14 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c > >> b/drivers/gpu/drm/bridge/dw-hdmi.c index 8a6a183..fa4147c 100644 > >> --- a/drivers/gpu/drm/bridge/dw-hdmi.c > >> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c > >> @@ -1420,8 +1420,11 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, > >> struct drm_display_mode *mode) > >> hdmi->hdmi_data.video_mode.mpixelrepetitionoutput = 0; > >> hdmi->hdmi_data.video_mode.mpixelrepetitioninput = 0; > >> > >> - /* TODO: Get input format from IPU (via FB driver interface) */ > >> - hdmi->hdmi_data.enc_in_format = RGB; > >> + /* Get input format from plat data or fallback to RGB */ > >> + if (hdmi->plat_data->input_fmt >= 0) > >> + hdmi->hdmi_data.enc_in_format = hdmi->plat_data->input_fmt; > > > > Also not a big fan of this approach, but its better than it was. > > BTW see relevant discussion about colorspace (this is more in the > > output path) here [1]. > > > > [1] https://patchwork.kernel.org/patch/9495153/ > > > >> + else > >> + hdmi->hdmi_data.enc_in_format = RGB; > >> > >> hdmi->hdmi_data.enc_out_format = RGB; > >> > >> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h > >> index d6a0ab3..4f426c3 100644 > >> --- a/include/drm/bridge/dw_hdmi.h > >> +++ b/include/drm/bridge/dw_hdmi.h > >> @@ -21,6 +21,14 @@ enum { > >> > >> DW_HDMI_RES_MAX, > >> > >> }; > >> > >> +enum { > >> + DW_HDMI_INPUT_FMT_RGB = 0, > >> + DW_HDMI_INPUT_FMT_YCBCR444, > >> + DW_HDMI_INPUT_FMT_YCBCR422_16BITS, > >> + DW_HDMI_INPUT_FMT_YCBCR422_8BITS, > > > > Hmm, did you test these two? I'm not sure if deep color can be > > converted using CSC. > > I simply copied the dw-hdmi internal values. > Proper handling of input formats capabilities and > output format pixel clock handling should be added sometime. > In the meantime, it's worth adding which input is supported. > > This approach is very limited, should I add a bitmask with all > support input formats and select between RGB, YUV444 or YUV422 > in dw_hdmi_setup ? Ideally the bridge mode set operation should be extended to take format and colorspace information (or another bridge operation should be created for that purpose, I'm still undecided). As that's quite a big change, I'm fine if you start by passing a fixed format and colorspace through platform data. I would however like the format to use the media bus format codes defined in include/uapi/linux/media-bus-format.h. As far as I'm aware we have no colorspace definitions in DRM, so we would need to fix that. V4L2 handles colorspaces, and the API went through several iterations before we got it working, with stupid mistakes that we don't want to repeat here. Jose pointed to https://patchwork.kernel.org/patch/9495153/ as a starting point, and I would like to point out the format and colorspace are two different things. A colorspace is a combination of an encoding and quantization and transfer functions. Hans Verkuil has researched this topic for V4L2 (see https://gstreamer.freedesktop.org/data/events/gstreamer-conference/2015/Hans Verkuil - Colorspaces and HDMI.pdf and https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/colorspaces.html), we *really* want to involve him in this discussion. I believe colorspace information should be shared between V4L2 and DRM the same way we share the media bus formats (We should have done so for 4CCs as well but it's unfortunately too late for that). > >> + DW_HDMI_INPUT_FMT_XVYCC444, > >> +}; > >> + > >> enum dw_hdmi_phy_type { > >> DW_HDMI_PHY_DWC_HDMI_TX_PHY = 0x00, > >> DW_HDMI_PHY_DWC_MHL_PHY_HEAC = 0xb2, > >> @@ -68,6 +76,7 @@ struct dw_hdmi_plat_data { > >> const struct dw_hdmi_plat_data *data); > >> bool (*hdmi_read_hpd)(struct dw_hdmi *hdmi, > >> const struct dw_hdmi_plat_data *data); > >> + int input_fmt; > >> }; > >> > >> int dw_hdmi_probe(struct platform_device *pdev, -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [RFC/RFT PATCH 4/4] drm/bridge: dw-hdmi: Take input format from plat_data Date: Wed, 18 Jan 2017 22:49:41 +0200 Message-ID: <3138084.ubGyLVHoXm@avalon> References: <1484656294-6140-1-git-send-email-narmstrong@baylibre.com> <66fc9acc-5db9-d124-fbe5-31893a5bd5df@baylibre.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [185.26.127.97]) by gabe.freedesktop.org (Postfix) with ESMTPS id A25E76E917 for ; Wed, 18 Jan 2017 20:49:23 +0000 (UTC) In-Reply-To: <66fc9acc-5db9-d124-fbe5-31893a5bd5df@baylibre.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Neil Armstrong Cc: Jose Abreu , laurent.pinchart+renesas@ideasonboard.com, kieran.bingham@ideasonboard.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Hans Verkuil , linux-amlogic@lists.infradead.org List-Id: dri-devel@lists.freedesktop.org SGkgTmVpbCwKCihDQydpbmcgSGFucyBWZXJrdWlsKQoKT24gV2VkbmVzZGF5IDE4IEphbiAyMDE3 IDEyOjI0OjIyIE5laWwgQXJtc3Ryb25nIHdyb3RlOgo+IE9uIDAxLzE4LzIwMTcgMTE6MjggQU0s IEpvc2UgQWJyZXUgd3JvdGU6Cj4gPiBPbiAxNy0wMS0yMDE3IDEyOjMxLCBOZWlsIEFybXN0cm9u ZyB3cm90ZToKPiA+PiBTb21lIGRpc3BsYXkgcGlwZWxpbmVzIGNhbiBvbmx5IHByb3ZpZGUgbm9u LVJCRyBpbnB1dCBwaXhlbHMgdG8gdGhlIEhETUkKPiA+PiBUWCBDb250cm9sbGVyLCB0aGlzIHBh dGNoIHRha2VzIHRoZSBwaXhlbCBmb3JtYXQgZnJvbSB0aGUgcGxhdF9kYXRhIGlmCj4gPj4gcHJv dmlkZWQuCj4gPj4gCj4gPj4gU2lnbmVkLW9mZi1ieTogTmVpbCBBcm1zdHJvbmcgPG5hcm1zdHJv bmdAYmF5bGlicmUuY29tPgo+ID4+IC0tLQo+ID4+IAo+ID4+ICBkcml2ZXJzL2dwdS9kcm0vYnJp ZGdlL2R3LWhkbWkuYyB8IDcgKysrKystLQo+ID4+ICBpbmNsdWRlL2RybS9icmlkZ2UvZHdfaGRt aS5oICAgICB8IDkgKysrKysrKysrCj4gPj4gIDIgZmlsZXMgY2hhbmdlZCwgMTQgaW5zZXJ0aW9u cygrKSwgMiBkZWxldGlvbnMoLSkKPiA+PiAKPiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUv ZHJtL2JyaWRnZS9kdy1oZG1pLmMKPiA+PiBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvZHctaGRt aS5jIGluZGV4IDhhNmExODMuLmZhNDE0N2MgMTAwNjQ0Cj4gPj4gLS0tIGEvZHJpdmVycy9ncHUv ZHJtL2JyaWRnZS9kdy1oZG1pLmMKPiA+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2R3 LWhkbWkuYwo+ID4+IEBAIC0xNDIwLDggKzE0MjAsMTEgQEAgc3RhdGljIGludCBkd19oZG1pX3Nl dHVwKHN0cnVjdCBkd19oZG1pICpoZG1pLAo+ID4+IHN0cnVjdCBkcm1fZGlzcGxheV9tb2RlICpt b2RlKQo+ID4+ICAJaGRtaS0+aGRtaV9kYXRhLnZpZGVvX21vZGUubXBpeGVscmVwZXRpdGlvbm91 dHB1dCA9IDA7Cj4gPj4gIAloZG1pLT5oZG1pX2RhdGEudmlkZW9fbW9kZS5tcGl4ZWxyZXBldGl0 aW9uaW5wdXQgPSAwOwo+ID4+IAo+ID4+IC0JLyogVE9ETzogR2V0IGlucHV0IGZvcm1hdCBmcm9t IElQVSAodmlhIEZCIGRyaXZlciBpbnRlcmZhY2UpICovCj4gPj4gLQloZG1pLT5oZG1pX2RhdGEu ZW5jX2luX2Zvcm1hdCA9IFJHQjsKPiA+PiArCS8qIEdldCBpbnB1dCBmb3JtYXQgZnJvbSBwbGF0 IGRhdGEgb3IgZmFsbGJhY2sgdG8gUkdCICovCj4gPj4gKwlpZiAoaGRtaS0+cGxhdF9kYXRhLT5p bnB1dF9mbXQgPj0gMCkKPiA+PiArCQloZG1pLT5oZG1pX2RhdGEuZW5jX2luX2Zvcm1hdCA9IGhk bWktPnBsYXRfZGF0YS0+aW5wdXRfZm10Owo+ID4gCj4gPiBBbHNvIG5vdCBhIGJpZyBmYW4gb2Yg dGhpcyBhcHByb2FjaCwgYnV0IGl0cyBiZXR0ZXIgdGhhbiBpdCB3YXMuCj4gPiBCVFcgc2VlIHJl bGV2YW50IGRpc2N1c3Npb24gYWJvdXQgY29sb3JzcGFjZSAodGhpcyBpcyBtb3JlIGluIHRoZQo+ ID4gb3V0cHV0IHBhdGgpIGhlcmUgWzFdLgo+ID4gCj4gPiBbMV0gaHR0cHM6Ly9wYXRjaHdvcmsu a2VybmVsLm9yZy9wYXRjaC85NDk1MTUzLwo+ID4gCj4gPj4gKwllbHNlCj4gPj4gKwkJaGRtaS0+ aGRtaV9kYXRhLmVuY19pbl9mb3JtYXQgPSBSR0I7Cj4gPj4gCj4gPj4gIAloZG1pLT5oZG1pX2Rh dGEuZW5jX291dF9mb3JtYXQgPSBSR0I7Cj4gPj4gCj4gPj4gZGlmZiAtLWdpdCBhL2luY2x1ZGUv ZHJtL2JyaWRnZS9kd19oZG1pLmggYi9pbmNsdWRlL2RybS9icmlkZ2UvZHdfaGRtaS5oCj4gPj4g aW5kZXggZDZhMGFiMy4uNGY0MjZjMyAxMDA2NDQKPiA+PiAtLS0gYS9pbmNsdWRlL2RybS9icmlk Z2UvZHdfaGRtaS5oCj4gPj4gKysrIGIvaW5jbHVkZS9kcm0vYnJpZGdlL2R3X2hkbWkuaAo+ID4+ IEBAIC0yMSw2ICsyMSwxNCBAQCBlbnVtIHsKPiA+PiAKPiA+PiAgCURXX0hETUlfUkVTX01BWCwK PiA+PiAgCj4gPj4gIH07Cj4gPj4gCj4gPj4gK2VudW0gewo+ID4+ICsJRFdfSERNSV9JTlBVVF9G TVRfUkdCID0gMCwKPiA+PiArCURXX0hETUlfSU5QVVRfRk1UX1lDQkNSNDQ0LAo+ID4+ICsJRFdf SERNSV9JTlBVVF9GTVRfWUNCQ1I0MjJfMTZCSVRTLAo+ID4+ICsJRFdfSERNSV9JTlBVVF9GTVRf WUNCQ1I0MjJfOEJJVFMsCj4gPiAKPiA+IEhtbSwgZGlkIHlvdSB0ZXN0IHRoZXNlIHR3bz8gSSdt IG5vdCBzdXJlIGlmIGRlZXAgY29sb3IgY2FuIGJlCj4gPiBjb252ZXJ0ZWQgdXNpbmcgQ1NDLgo+ IAo+IEkgc2ltcGx5IGNvcGllZCB0aGUgZHctaGRtaSBpbnRlcm5hbCB2YWx1ZXMuCj4gUHJvcGVy IGhhbmRsaW5nIG9mIGlucHV0IGZvcm1hdHMgY2FwYWJpbGl0aWVzIGFuZAo+IG91dHB1dCBmb3Jt YXQgcGl4ZWwgY2xvY2sgaGFuZGxpbmcgc2hvdWxkIGJlIGFkZGVkIHNvbWV0aW1lLgo+IEluIHRo ZSBtZWFudGltZSwgaXQncyB3b3J0aCBhZGRpbmcgd2hpY2ggaW5wdXQgaXMgc3VwcG9ydGVkLgo+ IAo+IFRoaXMgYXBwcm9hY2ggaXMgdmVyeSBsaW1pdGVkLCBzaG91bGQgSSBhZGQgYSBiaXRtYXNr IHdpdGggYWxsCj4gc3VwcG9ydCBpbnB1dCBmb3JtYXRzIGFuZCBzZWxlY3QgYmV0d2VlbiBSR0Is IFlVVjQ0NCBvciBZVVY0MjIKPiBpbiBkd19oZG1pX3NldHVwID8KCklkZWFsbHkgdGhlIGJyaWRn ZSBtb2RlIHNldCBvcGVyYXRpb24gc2hvdWxkIGJlIGV4dGVuZGVkIHRvIHRha2UgZm9ybWF0IGFu ZCAKY29sb3JzcGFjZSBpbmZvcm1hdGlvbiAob3IgYW5vdGhlciBicmlkZ2Ugb3BlcmF0aW9uIHNo b3VsZCBiZSBjcmVhdGVkIGZvciB0aGF0IApwdXJwb3NlLCBJJ20gc3RpbGwgdW5kZWNpZGVkKS4g QXMgdGhhdCdzIHF1aXRlIGEgYmlnIGNoYW5nZSwgSSdtIGZpbmUgaWYgeW91IApzdGFydCBieSBw YXNzaW5nIGEgZml4ZWQgZm9ybWF0IGFuZCBjb2xvcnNwYWNlIHRocm91Z2ggcGxhdGZvcm0gZGF0 YS4gSSB3b3VsZCAKaG93ZXZlciBsaWtlIHRoZSBmb3JtYXQgdG8gdXNlIHRoZSBtZWRpYSBidXMg Zm9ybWF0IGNvZGVzIGRlZmluZWQgaW4gCmluY2x1ZGUvdWFwaS9saW51eC9tZWRpYS1idXMtZm9y bWF0LmguCgpBcyBmYXIgYXMgSSdtIGF3YXJlIHdlIGhhdmUgbm8gY29sb3JzcGFjZSBkZWZpbml0 aW9ucyBpbiBEUk0sIHNvIHdlIHdvdWxkIG5lZWQgCnRvIGZpeCB0aGF0LiBWNEwyIGhhbmRsZXMg Y29sb3JzcGFjZXMsIGFuZCB0aGUgQVBJIHdlbnQgdGhyb3VnaCBzZXZlcmFsIAppdGVyYXRpb25z IGJlZm9yZSB3ZSBnb3QgaXQgd29ya2luZywgd2l0aCBzdHVwaWQgbWlzdGFrZXMgdGhhdCB3ZSBk b24ndCB3YW50IAp0byByZXBlYXQgaGVyZS4KCkpvc2UgcG9pbnRlZCB0byBodHRwczovL3BhdGNo d29yay5rZXJuZWwub3JnL3BhdGNoLzk0OTUxNTMvIGFzIGEgc3RhcnRpbmcgCnBvaW50LCBhbmQg SSB3b3VsZCBsaWtlIHRvIHBvaW50IG91dCB0aGUgZm9ybWF0IGFuZCBjb2xvcnNwYWNlIGFyZSB0 d28gCmRpZmZlcmVudCB0aGluZ3MuIEEgY29sb3JzcGFjZSBpcyBhIGNvbWJpbmF0aW9uIG9mIGFu IGVuY29kaW5nIGFuZCAgCnF1YW50aXphdGlvbiBhbmQgdHJhbnNmZXIgZnVuY3Rpb25zLiBIYW5z IFZlcmt1aWwgaGFzIHJlc2VhcmNoZWQgdGhpcyB0b3BpYyAKZm9yIFY0TDIgKHNlZSBodHRwczov L2dzdHJlYW1lci5mcmVlZGVza3RvcC5vcmcvZGF0YS9ldmVudHMvZ3N0cmVhbWVyLWNvbmZlcmVu Y2UvMjAxNS9IYW5zIFZlcmt1aWwgLSBDb2xvcnNwYWNlcyBhbmQgSERNSS5wZGYgYW5kIApodHRw czovL2xpbnV4dHYub3JnL2Rvd25sb2Fkcy92NGwtZHZiLWFwaXMvdWFwaS92NGwvY29sb3JzcGFj ZXMuaHRtbCksIHdlIAoqcmVhbGx5KiB3YW50IHRvIGludm9sdmUgaGltIGluIHRoaXMgZGlzY3Vz c2lvbi4KCkkgYmVsaWV2ZSBjb2xvcnNwYWNlIGluZm9ybWF0aW9uIHNob3VsZCBiZSBzaGFyZWQg YmV0d2VlbiBWNEwyIGFuZCBEUk0gdGhlIApzYW1lIHdheSB3ZSBzaGFyZSB0aGUgbWVkaWEgYnVz IGZvcm1hdHMgKFdlIHNob3VsZCBoYXZlIGRvbmUgc28gZm9yIDRDQ3MgYXMgCndlbGwgYnV0IGl0 J3MgdW5mb3J0dW5hdGVseSB0b28gbGF0ZSBmb3IgdGhhdCkuCgo+ID4+ICsJRFdfSERNSV9JTlBV VF9GTVRfWFZZQ0M0NDQsCj4gPj4gK307Cj4gPj4gKwo+ID4+ICBlbnVtIGR3X2hkbWlfcGh5X3R5 cGUgewo+ID4+ICAJRFdfSERNSV9QSFlfRFdDX0hETUlfVFhfUEhZID0gMHgwMCwKPiA+PiAgCURX X0hETUlfUEhZX0RXQ19NSExfUEhZX0hFQUMgPSAweGIyLAo+ID4+IEBAIC02OCw2ICs3Niw3IEBA IHN0cnVjdCBkd19oZG1pX3BsYXRfZGF0YSB7Cj4gPj4gIAkJCQkgY29uc3Qgc3RydWN0IGR3X2hk bWlfcGxhdF9kYXRhICpkYXRhKTsKPiA+PiAgCWJvb2wgKCpoZG1pX3JlYWRfaHBkKShzdHJ1Y3Qg ZHdfaGRtaSAqaGRtaSwKPiA+PiAgCQkJICAgICAgY29uc3Qgc3RydWN0IGR3X2hkbWlfcGxhdF9k YXRhICpkYXRhKTsKPiA+PiArCWludCBpbnB1dF9mbXQ7Cj4gPj4gIH07Cj4gPj4gIAo+ID4+ICBp bnQgZHdfaGRtaV9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2LAoKLS0gClJlZ2Fy ZHMsCgpMYXVyZW50IFBpbmNoYXJ0CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVl ZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5m by9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752802AbdARUuM (ORCPT ); Wed, 18 Jan 2017 15:50:12 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:45010 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751619AbdARUuG (ORCPT ); Wed, 18 Jan 2017 15:50:06 -0500 From: Laurent Pinchart To: Neil Armstrong Cc: Jose Abreu , dri-devel@lists.freedesktop.org, laurent.pinchart+renesas@ideasonboard.com, kieran.bingham@ideasonboard.com, linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org, Hans Verkuil Subject: Re: [RFC/RFT PATCH 4/4] drm/bridge: dw-hdmi: Take input format from plat_data Date: Wed, 18 Jan 2017 22:49:41 +0200 Message-ID: <3138084.ubGyLVHoXm@avalon> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: <66fc9acc-5db9-d124-fbe5-31893a5bd5df@baylibre.com> References: <1484656294-6140-1-git-send-email-narmstrong@baylibre.com> <66fc9acc-5db9-d124-fbe5-31893a5bd5df@baylibre.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Neil, (CC'ing Hans Verkuil) On Wednesday 18 Jan 2017 12:24:22 Neil Armstrong wrote: > On 01/18/2017 11:28 AM, Jose Abreu wrote: > > On 17-01-2017 12:31, Neil Armstrong wrote: > >> Some display pipelines can only provide non-RBG input pixels to the HDMI > >> TX Controller, this patch takes the pixel format from the plat_data if > >> provided. > >> > >> Signed-off-by: Neil Armstrong > >> --- > >> > >> drivers/gpu/drm/bridge/dw-hdmi.c | 7 +++++-- > >> include/drm/bridge/dw_hdmi.h | 9 +++++++++ > >> 2 files changed, 14 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c > >> b/drivers/gpu/drm/bridge/dw-hdmi.c index 8a6a183..fa4147c 100644 > >> --- a/drivers/gpu/drm/bridge/dw-hdmi.c > >> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c > >> @@ -1420,8 +1420,11 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, > >> struct drm_display_mode *mode) > >> hdmi->hdmi_data.video_mode.mpixelrepetitionoutput = 0; > >> hdmi->hdmi_data.video_mode.mpixelrepetitioninput = 0; > >> > >> - /* TODO: Get input format from IPU (via FB driver interface) */ > >> - hdmi->hdmi_data.enc_in_format = RGB; > >> + /* Get input format from plat data or fallback to RGB */ > >> + if (hdmi->plat_data->input_fmt >= 0) > >> + hdmi->hdmi_data.enc_in_format = hdmi->plat_data->input_fmt; > > > > Also not a big fan of this approach, but its better than it was. > > BTW see relevant discussion about colorspace (this is more in the > > output path) here [1]. > > > > [1] https://patchwork.kernel.org/patch/9495153/ > > > >> + else > >> + hdmi->hdmi_data.enc_in_format = RGB; > >> > >> hdmi->hdmi_data.enc_out_format = RGB; > >> > >> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h > >> index d6a0ab3..4f426c3 100644 > >> --- a/include/drm/bridge/dw_hdmi.h > >> +++ b/include/drm/bridge/dw_hdmi.h > >> @@ -21,6 +21,14 @@ enum { > >> > >> DW_HDMI_RES_MAX, > >> > >> }; > >> > >> +enum { > >> + DW_HDMI_INPUT_FMT_RGB = 0, > >> + DW_HDMI_INPUT_FMT_YCBCR444, > >> + DW_HDMI_INPUT_FMT_YCBCR422_16BITS, > >> + DW_HDMI_INPUT_FMT_YCBCR422_8BITS, > > > > Hmm, did you test these two? I'm not sure if deep color can be > > converted using CSC. > > I simply copied the dw-hdmi internal values. > Proper handling of input formats capabilities and > output format pixel clock handling should be added sometime. > In the meantime, it's worth adding which input is supported. > > This approach is very limited, should I add a bitmask with all > support input formats and select between RGB, YUV444 or YUV422 > in dw_hdmi_setup ? Ideally the bridge mode set operation should be extended to take format and colorspace information (or another bridge operation should be created for that purpose, I'm still undecided). As that's quite a big change, I'm fine if you start by passing a fixed format and colorspace through platform data. I would however like the format to use the media bus format codes defined in include/uapi/linux/media-bus-format.h. As far as I'm aware we have no colorspace definitions in DRM, so we would need to fix that. V4L2 handles colorspaces, and the API went through several iterations before we got it working, with stupid mistakes that we don't want to repeat here. Jose pointed to https://patchwork.kernel.org/patch/9495153/ as a starting point, and I would like to point out the format and colorspace are two different things. A colorspace is a combination of an encoding and quantization and transfer functions. Hans Verkuil has researched this topic for V4L2 (see https://gstreamer.freedesktop.org/data/events/gstreamer-conference/2015/Hans Verkuil - Colorspaces and HDMI.pdf and https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/colorspaces.html), we *really* want to involve him in this discussion. I believe colorspace information should be shared between V4L2 and DRM the same way we share the media bus formats (We should have done so for 4CCs as well but it's unfortunately too late for that). > >> + DW_HDMI_INPUT_FMT_XVYCC444, > >> +}; > >> + > >> enum dw_hdmi_phy_type { > >> DW_HDMI_PHY_DWC_HDMI_TX_PHY = 0x00, > >> DW_HDMI_PHY_DWC_MHL_PHY_HEAC = 0xb2, > >> @@ -68,6 +76,7 @@ struct dw_hdmi_plat_data { > >> const struct dw_hdmi_plat_data *data); > >> bool (*hdmi_read_hpd)(struct dw_hdmi *hdmi, > >> const struct dw_hdmi_plat_data *data); > >> + int input_fmt; > >> }; > >> > >> int dw_hdmi_probe(struct platform_device *pdev, -- Regards, Laurent Pinchart