From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 798F7C4321E for ; Wed, 23 Feb 2022 16:57:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8CD6710F1BF; Wed, 23 Feb 2022 16:57:40 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) by gabe.freedesktop.org (Postfix) with ESMTPS id AFEA010F1BF for ; Wed, 23 Feb 2022 16:57:39 +0000 (UTC) Received: from [127.0.0.1] (p578adb1c.dip0.t-ipconnect.de [87.138.219.28]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 3EDFA83CA0; Wed, 23 Feb 2022 17:57:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1645635457; bh=6RNnxFry+EgzWhSv2OUCDvWAJ7ot/hbdOI28iSxKcJU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=g5QTKwgLA6VEINSzj3IjkjmC/0f158foiTe59tKknQKbbLE0nPbKSM+/nvRri7fkL WrHWYj6ArDm8lw/DhCdM4zJMZVFO08bKvTqBd7bJoZFpxDTex6mxCEXNykTMgDf0iy eJ90IF5FWQbDxvgd2/rIU7djiB+tK7RthWG/s35kBqT+DLyZw3QQ222jmullQOBo89 UnnNVWdEksRSD2T9PhsWRyNOuIMemVR6LmrTKPdCNdouI4VIcaJwULVeApKdn1uIJE iu8mEYb62NcJzDcnC7dnjT3wuVAGckueTPExnISxH0sBNDbx6WzntkWuVcYWqvHw5w KDHW6fw2kM1AA== Message-ID: Date: Wed, 23 Feb 2022 17:57:35 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.0 Subject: Re: [RFC PATCH] drm/panel: simple: panel-dpi: use bus-format to set bpc and bus_format Content-Language: en-US To: Maxime Ripard References: <20220222084723.14310-1-max.krummenacher@toradex.com> <20220223134154.oo7xhf37bgtvm3ai@houat> <20220223134757.f5upi2iun27op5w5@houat> <20220223143703.xi7vpamjg4ytmvqs@houat> <20220223163930.wk3twgz6hranicv6@houat> From: Marek Vasut In-Reply-To: <20220223163930.wk3twgz6hranicv6@houat> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Christoph Niedermaier , Max Krummenacher , Pengutronix Kernel Team , Max Krummenacher , David Airlie , Sam Ravnborg , Sascha Hauer , dri-devel@lists.freedesktop.org, DenysDrozdov , Laurent Pinchart , Shawn Guo , linux-arm-kernel@lists.infradead.org, NXP Linux Team Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 2/23/22 17:39, Maxime Ripard wrote: > On Wed, Feb 23, 2022 at 03:38:20PM +0100, Marek Vasut wrote: >> On 2/23/22 15:37, Maxime Ripard wrote: >>> On Wed, Feb 23, 2022 at 03:09:08PM +0100, Marek Vasut wrote: >>>> On 2/23/22 14:47, Maxime Ripard wrote: >>>>> On Wed, Feb 23, 2022 at 02:45:30PM +0100, Marek Vasut wrote: >>>>>> On 2/23/22 14:41, Maxime Ripard wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On Tue, Feb 22, 2022 at 09:47:23AM +0100, Max Krummenacher wrote: >>>>>>>> Use the new property bus-format to set the enum bus_format and bpc. >>>>>>>> Completes: commit 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support") >>>>>>>> >>>>>>>> Signed-off-by: Max Krummenacher >>>>>>>> >>>>>>>> --- >>>>>>>> >>>>>>>> drivers/gpu/drm/panel/panel-simple.c | 32 ++++++++++++++++++++++++++++ >>>>>>>> 1 file changed, 32 insertions(+) >>>>>>>> >>>>>>>> Relates to the discussion: https://lore.kernel.org/all/20220201110717.3585-1-cniedermaier@dh-electronics.com/ >>>>>>>> >>>>>>>> Max >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> index c5f133667a2d..5c07260de71c 100644 >>>>>>>> --- a/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> +++ b/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> @@ -453,6 +453,7 @@ static int panel_dpi_probe(struct device *dev, >>>>>>>> struct panel_desc *desc; >>>>>>>> unsigned int bus_flags; >>>>>>>> struct videomode vm; >>>>>>>> + const char *format = ""; >>>>>>>> int ret; >>>>>>>> np = dev->of_node; >>>>>>>> @@ -477,6 +478,37 @@ static int panel_dpi_probe(struct device *dev, >>>>>>>> of_property_read_u32(np, "width-mm", &desc->size.width); >>>>>>>> of_property_read_u32(np, "height-mm", &desc->size.height); >>>>>>>> + of_property_read_string(np, "bus-format", &format); >>>>>>>> + if (!strcmp(format, "BGR888_1X24")) { >>>>>>>> + desc->bpc = 8; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_BGR888_1X24; >>>>>>>> + } else if (!strcmp(format, "GBR888_1X24")) { >>>>>>>> + desc->bpc = 8; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_GBR888_1X24; >>>>>>>> + } else if (!strcmp(format, "RBG888_1X24")) { >>>>>>>> + desc->bpc = 8; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RBG888_1X24; >>>>>>>> + } else if (!strcmp(format, "RGB444_1X12")) { >>>>>>>> + desc->bpc = 6; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB444_1X12; >>>>>>>> + } else if (!strcmp(format, "RGB565_1X16")) { >>>>>>>> + desc->bpc = 6; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; >>>>>>>> + } else if (!strcmp(format, "RGB666_1X18")) { >>>>>>>> + desc->bpc = 6; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; >>>>>>>> + } else if (!strcmp(format, "RGB666_1X24_CPADHI")) { >>>>>>>> + desc->bpc = 6; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; >>>>>>>> + } else if (!strcmp(format, "RGB888_1X24")) { >>>>>>>> + desc->bpc = 8; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; >>>>>>>> + } else { >>>>>>>> + dev_err(dev, "%pOF: missing or unknown bus-format property\n", >>>>>>>> + np); >>>>>>>> + return -EINVAL; >>>>>>>> + } >>>>>>>> + >>>>>>> >>>>>>> It doesn't seem right, really. We can't the bus format / bpc be inferred >>>>>>> from the compatible? I'd expect two panels that don't have the same bus >>>>>>> format to not be claimed as compatible. >>>>>> >>>>>> Which compatible ? >>>>>> >>>>>> Note that this is for panel-dpi compatible, i.e. the panel which has timings >>>>>> specified in DT (and needs bus format specified there too). >>>>> >>>>> panel-dpi is supposed to have two compatibles, the panel-specific one >>>>> and panel-dpi. This would obviously be tied to the panel-specific one. >>>> >>>> This whole discussion is about the one which only has 'panel-dpi' compatible >>>> and describes the panel in DT completely. The specific compatible is not >>>> present in DT when this patch is needed. >>> >>> From the panel-dpi DT binding: >>> >>> properties: >>> compatible: >>> description: >>> Shall contain a panel specific compatible and "panel-dpi" >>> in that order. >>> items: >>> - {} >>> - const: panel-dpi >>> >>> The panel-specific compatible is mandatory, whether you like it or not. >> >> It doesn't seem to me that's the intended use per panel-simple.c , so maybe >> the bindings need to be fixed too ? > > It's not clear to me why this has anything to do with panel-simple, but > the binding is correct, and that requirement is fairly standard. We have > the same thing with panel-lvds for example. I think this patch is related to this patch, which was not mentioned in the commit message: [RFC][PATCH] Revert "drm/panel-simple: drop use of data-mapping property" (unless I am confused) With LVDS the situation is simpler, you have three formats and that's it (18bpp and 2 24bpp), with DPI it is more complex, since you need to deal with color channel width (888, 666 and even 565 and other oddities), then with mapping (RGB, BGR, etc), and then you can have panels with only 18bpp inputs wired to 24bpp DPI bus and vice versa which you also have to somehow describe. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E9CB1C433EF for ; Wed, 23 Feb 2022 16:59:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=HIyvIniEgDqA0eVpU8GaRdRqOqHLpNaF5k804JRMMzQ=; b=gAS78lHMpADQEe 7x7iAuYntn7A5kX9Y+uNyiDiIZBtSx1LGRWOlVePxfwmCjYFnV41SCmdegiozEGG/YIFbG9NdNf7G TjLvqK73N4TCxLMpipNJS/O6iPXwFiOkv8l/ciPhQhseU2e8bxgNy/YnUxjUB5jEWiFMYInD7pyR1 n7hfwZdwD5Nmz3C2kh01v/esZ1ACjf2ZxPQ5VYHWqzYzUvnEElA7exw+M2cTTsguTLRo3rk5ukhaH EEWZzomyYUNjafKmoVZDE2wlMvgPF0jV2A1e+6QbJE4PXvdQWF32oBZ47ZEj8CzMP/viPT+lkacjj 1lSH6t6EOx6/n44NuDCA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nMuxU-00FPTI-Ke; Wed, 23 Feb 2022 16:57:44 +0000 Received: from phobos.denx.de ([85.214.62.61]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nMuxP-00FPQx-US for linux-arm-kernel@lists.infradead.org; Wed, 23 Feb 2022 16:57:42 +0000 Received: from [127.0.0.1] (p578adb1c.dip0.t-ipconnect.de [87.138.219.28]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 3EDFA83CA0; Wed, 23 Feb 2022 17:57:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1645635457; bh=6RNnxFry+EgzWhSv2OUCDvWAJ7ot/hbdOI28iSxKcJU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=g5QTKwgLA6VEINSzj3IjkjmC/0f158foiTe59tKknQKbbLE0nPbKSM+/nvRri7fkL WrHWYj6ArDm8lw/DhCdM4zJMZVFO08bKvTqBd7bJoZFpxDTex6mxCEXNykTMgDf0iy eJ90IF5FWQbDxvgd2/rIU7djiB+tK7RthWG/s35kBqT+DLyZw3QQ222jmullQOBo89 UnnNVWdEksRSD2T9PhsWRyNOuIMemVR6LmrTKPdCNdouI4VIcaJwULVeApKdn1uIJE iu8mEYb62NcJzDcnC7dnjT3wuVAGckueTPExnISxH0sBNDbx6WzntkWuVcYWqvHw5w KDHW6fw2kM1AA== Message-ID: Date: Wed, 23 Feb 2022 17:57:35 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.0 Subject: Re: [RFC PATCH] drm/panel: simple: panel-dpi: use bus-format to set bpc and bus_format Content-Language: en-US To: Maxime Ripard Cc: Max Krummenacher , dri-devel@lists.freedesktop.org, Sascha Hauer , Philipp Zabel , Laurent Pinchart , Fabio Estevam , linux-arm-kernel@lists.infradead.org, DenysDrozdov , David Airlie , Christoph Niedermaier , Pengutronix Kernel Team , Sam Ravnborg , Shawn Guo , Daniel Vetter , NXP Linux Team , Max Krummenacher References: <20220222084723.14310-1-max.krummenacher@toradex.com> <20220223134154.oo7xhf37bgtvm3ai@houat> <20220223134757.f5upi2iun27op5w5@houat> <20220223143703.xi7vpamjg4ytmvqs@houat> <20220223163930.wk3twgz6hranicv6@houat> From: Marek Vasut In-Reply-To: <20220223163930.wk3twgz6hranicv6@houat> X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220223_085740_316794_70233A85 X-CRM114-Status: GOOD ( 23.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2/23/22 17:39, Maxime Ripard wrote: > On Wed, Feb 23, 2022 at 03:38:20PM +0100, Marek Vasut wrote: >> On 2/23/22 15:37, Maxime Ripard wrote: >>> On Wed, Feb 23, 2022 at 03:09:08PM +0100, Marek Vasut wrote: >>>> On 2/23/22 14:47, Maxime Ripard wrote: >>>>> On Wed, Feb 23, 2022 at 02:45:30PM +0100, Marek Vasut wrote: >>>>>> On 2/23/22 14:41, Maxime Ripard wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On Tue, Feb 22, 2022 at 09:47:23AM +0100, Max Krummenacher wrote: >>>>>>>> Use the new property bus-format to set the enum bus_format and bpc. >>>>>>>> Completes: commit 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support") >>>>>>>> >>>>>>>> Signed-off-by: Max Krummenacher >>>>>>>> >>>>>>>> --- >>>>>>>> >>>>>>>> drivers/gpu/drm/panel/panel-simple.c | 32 ++++++++++++++++++++++++++++ >>>>>>>> 1 file changed, 32 insertions(+) >>>>>>>> >>>>>>>> Relates to the discussion: https://lore.kernel.org/all/20220201110717.3585-1-cniedermaier@dh-electronics.com/ >>>>>>>> >>>>>>>> Max >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> index c5f133667a2d..5c07260de71c 100644 >>>>>>>> --- a/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> +++ b/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> @@ -453,6 +453,7 @@ static int panel_dpi_probe(struct device *dev, >>>>>>>> struct panel_desc *desc; >>>>>>>> unsigned int bus_flags; >>>>>>>> struct videomode vm; >>>>>>>> + const char *format = ""; >>>>>>>> int ret; >>>>>>>> np = dev->of_node; >>>>>>>> @@ -477,6 +478,37 @@ static int panel_dpi_probe(struct device *dev, >>>>>>>> of_property_read_u32(np, "width-mm", &desc->size.width); >>>>>>>> of_property_read_u32(np, "height-mm", &desc->size.height); >>>>>>>> + of_property_read_string(np, "bus-format", &format); >>>>>>>> + if (!strcmp(format, "BGR888_1X24")) { >>>>>>>> + desc->bpc = 8; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_BGR888_1X24; >>>>>>>> + } else if (!strcmp(format, "GBR888_1X24")) { >>>>>>>> + desc->bpc = 8; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_GBR888_1X24; >>>>>>>> + } else if (!strcmp(format, "RBG888_1X24")) { >>>>>>>> + desc->bpc = 8; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RBG888_1X24; >>>>>>>> + } else if (!strcmp(format, "RGB444_1X12")) { >>>>>>>> + desc->bpc = 6; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB444_1X12; >>>>>>>> + } else if (!strcmp(format, "RGB565_1X16")) { >>>>>>>> + desc->bpc = 6; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; >>>>>>>> + } else if (!strcmp(format, "RGB666_1X18")) { >>>>>>>> + desc->bpc = 6; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; >>>>>>>> + } else if (!strcmp(format, "RGB666_1X24_CPADHI")) { >>>>>>>> + desc->bpc = 6; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; >>>>>>>> + } else if (!strcmp(format, "RGB888_1X24")) { >>>>>>>> + desc->bpc = 8; >>>>>>>> + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; >>>>>>>> + } else { >>>>>>>> + dev_err(dev, "%pOF: missing or unknown bus-format property\n", >>>>>>>> + np); >>>>>>>> + return -EINVAL; >>>>>>>> + } >>>>>>>> + >>>>>>> >>>>>>> It doesn't seem right, really. We can't the bus format / bpc be inferred >>>>>>> from the compatible? I'd expect two panels that don't have the same bus >>>>>>> format to not be claimed as compatible. >>>>>> >>>>>> Which compatible ? >>>>>> >>>>>> Note that this is for panel-dpi compatible, i.e. the panel which has timings >>>>>> specified in DT (and needs bus format specified there too). >>>>> >>>>> panel-dpi is supposed to have two compatibles, the panel-specific one >>>>> and panel-dpi. This would obviously be tied to the panel-specific one. >>>> >>>> This whole discussion is about the one which only has 'panel-dpi' compatible >>>> and describes the panel in DT completely. The specific compatible is not >>>> present in DT when this patch is needed. >>> >>> From the panel-dpi DT binding: >>> >>> properties: >>> compatible: >>> description: >>> Shall contain a panel specific compatible and "panel-dpi" >>> in that order. >>> items: >>> - {} >>> - const: panel-dpi >>> >>> The panel-specific compatible is mandatory, whether you like it or not. >> >> It doesn't seem to me that's the intended use per panel-simple.c , so maybe >> the bindings need to be fixed too ? > > It's not clear to me why this has anything to do with panel-simple, but > the binding is correct, and that requirement is fairly standard. We have > the same thing with panel-lvds for example. I think this patch is related to this patch, which was not mentioned in the commit message: [RFC][PATCH] Revert "drm/panel-simple: drop use of data-mapping property" (unless I am confused) With LVDS the situation is simpler, you have three formats and that's it (18bpp and 2 24bpp), with DPI it is more complex, since you need to deal with color channel width (888, 666 and even 565 and other oddities), then with mapping (RGB, BGR, etc), and then you can have panels with only 18bpp inputs wired to 24bpp DPI bus and vice versa which you also have to somehow describe. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel