Message ID | 20231023211158.424489-1-jonas@kwiboo.se |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce89:0:b0:403:3b70:6f57 with SMTP id p9csp1556500vqx; Mon, 23 Oct 2023 14:12:28 -0700 (PDT) X-Google-Smtp-Source: AGHT+IERcL5yOk1M105+Z6MNiSYDEcxlmq4zKLMLWdF+v4SBPDvMAJJGNKSp1lHFodEL5J556Z5H X-Received: by 2002:a17:902:eccb:b0:1c9:ba18:785c with SMTP id a11-20020a170902eccb00b001c9ba18785cmr10281864plh.25.1698095548685; Mon, 23 Oct 2023 14:12:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1698095548; cv=none; d=google.com; s=arc-20160816; b=O9Tx8ibjJNAanQUjP3n+fweJwsgH3DzD0j3DdLcpxhaU66vqo9MiCy3/ZfP+83aYmv unLB0idjVpHYhoVtk7znxD0DI8Cz8mIrN5bl2zZOhC8vCrE0X9He2l2iPpB8t5aLmQ6Z 5XpiRbjAKT38BRvV+DJMDPkZ4FBqlLaJmYHniPI/AN2IdTW54Ph/PtaZQ3tvNsTvqqH5 GgUwqWRykI0hYP83WagZUM5wGHQoU4aDNxbx/rWtDmWDsMQpkNpYd55mKaXVb/M1Ux6q ldtPtScRDzwOAXYGxAy+OkWnPIYJuvMjs1BzcIOl26NOAyw/Iw3K0YvYF8IgUw9ZVBEC Bahg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=EPCnxjTdpRw5P9F35T3qjQzzJlRqsS8bWZ4roLB5nXo=; fh=lVcdZksmXWbmm7xi9ImC/wKtTpdQKgs1fiJFZe6Q3JI=; b=09/DXsCy+Az6HwR6Pz+fowNkbqaiVqg5Jt5PNQgIVmAyhWFz5DWU0qSzdXq25bEJIg vGVa4/bxDntkEAz+UDwIUh0AkWR4/+Aui7sNnNspnD5S9LFRm/lBDKHfySZRJzGEiJl/ Dvbc6EATKO10MVjmxu66tfK73M4DD/Kmc0c40B/aiUE4OKU1qhuYD2FKDucAA3DOVuup 2WeOzrOJlMrUQbsfCl8AYXPZ+n/mAuEsV/IJoVn0+NNQfVgFJ70vowksHbJo1blZsgFH hQOlN8360kvkuh24LQBdIJfXQ+P9cSzP6ffB7DcSw7qm9Mgm0blOStqmZT6hP2emZUhw sCBA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kwiboo.se header.s=fe-e1b5cab7be header.b=gcNihjn2; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kwiboo.se Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id u13-20020a170902e80d00b001bb9e2c38ecsi57804plg.264.2023.10.23.14.12.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Oct 2023 14:12:28 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@kwiboo.se header.s=fe-e1b5cab7be header.b=gcNihjn2; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kwiboo.se Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id A992B8032178; Mon, 23 Oct 2023 14:12:26 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229879AbjJWVMS (ORCPT <rfc822;aposhian.dev@gmail.com> + 27 others); Mon, 23 Oct 2023 17:12:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50316 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229499AbjJWVMR (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 23 Oct 2023 17:12:17 -0400 Received: from smtp.forwardemail.net (smtp.forwardemail.net [149.28.215.223]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E115FA4 for <linux-kernel@vger.kernel.org>; Mon, 23 Oct 2023 14:12:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kwiboo.se; h=Content-Transfer-Encoding: MIME-Version: Message-ID: Date: Subject: Cc: To: From; q=dns/txt; s=fe-e1b5cab7be; t=1698095525; bh=EPCnxjTdpRw5P9F35T3qjQzzJlRqsS8bWZ4roLB5nXo=; b=gcNihjn2SE/Kg7pcvFvXdA5rphQHLWmJseBfsBDen6uO1LZNX9As3uLIo0NGIeX7FWOveDGzd 8AIwr2HbiQ7yo5pMNC++MjF5rF5XwVvW9jYd2+FWB4jvvoZjtxi5aS50GM9S+C1kprBR0VrF7WC HSrG1GKcFM4qPOOXlivj3DAClfqplBnlG1k6uUK+Spz0g3SeK6BnuLoT/THn+8EbUVG/ioFtm3j Bc/G9KDZrlAy6xcU8SG3ROR/BkTzpKQeYnuCT1kounkx5T64lxFVMgHz34pe2O+XMEblFf3yNiU 0HhtAKwsgC2iPChC7PKS8JGP6DJbp7qq/6wpF57upODA== From: Jonas Karlman <jonas@kwiboo.se> To: Heiko Stuebner <heiko@sntech.de>, Sandy Huang <hjc@rock-chips.com>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch> Cc: Christopher Obbard <chris.obbard@collabora.com>, Jonas Karlman <jonas@kwiboo.se>, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH] drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full Date: Mon, 23 Oct 2023 21:11:57 +0000 Message-ID: <20231023211158.424489-1-jonas@kwiboo.se> X-Mailer: git-send-email 2.42.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Report-Abuse-To: abuse@forwardemail.net X-Report-Abuse: abuse@forwardemail.net X-Complaints-To: abuse@forwardemail.net X-ForwardEmail-Version: 0.4.40 X-ForwardEmail-Sender: rfc822; jonas@kwiboo.se, smtp.forwardemail.net, 149.28.215.223 X-ForwardEmail-ID: 6536e1a4b7c1916c16a25195 X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Mon, 23 Oct 2023 14:12:26 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780582237808166210 X-GMAIL-MSGID: 1780582237808166210 |
Series |
drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full
|
|
Commit Message
Jonas Karlman
Oct. 23, 2023, 9:11 p.m. UTC
Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328
and RK3399 result in wrong colors being displayed.
The issue can be observed using modetest:
modetest -s <connector_id>@<crtc_id>:1920x1080-60@RG24
modetest -s <connector_id>@<crtc_id>:1920x1080-60@BG24
Vendor 4.4 kernel apply an inverted rb swap for these formats on VOP
full (major = 3).
Fix colors by applying rb swap similar to vendor 4.4 kernel.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
Comments
Hi Jonas, On Mon, 2023-10-23 at 21:11 +0000, Jonas Karlman wrote: > Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328 > and RK3399 result in wrong colors being displayed. > > The issue can be observed using modetest: > > modetest -s <connector_id>@<crtc_id>:1920x1080-60@RG24 > modetest -s <connector_id>@<crtc_id>:1920x1080-60@BG24 > > Vendor 4.4 kernel apply an inverted rb swap for these formats on VOP > full (major = 3). > > Fix colors by applying rb swap similar to vendor 4.4 kernel. > > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> How about a fixes tag? Seems like this was introduced in commit 85a359f25388 ("drm/rockchip: Add BGR formats to VOP") > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index b3d0b6ae9294..a1ce09a22f83 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -247,14 +247,17 @@ static inline void vop_cfg_done(struct vop *vop) > VOP_REG_SET(vop, common, cfg_done, 1); > } > > -static bool has_rb_swapped(uint32_t format) > +static bool has_rb_swapped(uint32_t version, uint32_t format) > { > switch (format) { > case DRM_FORMAT_XBGR8888: > case DRM_FORMAT_ABGR8888: > - case DRM_FORMAT_BGR888: > case DRM_FORMAT_BGR565: > return true; > + case DRM_FORMAT_RGB888: > + return VOP_MAJOR(version) == 3; > + case DRM_FORMAT_BGR888: > + return VOP_MAJOR(version) != 3; The hardcoded bits are quite scary as it applies to all hardware variants ;-). Is it worth adding an inline comment to describe what is going on and how it relates to VOPL and VOPH? Or would it be even better to add this as a quirk in the various vop_data structs? Cheers! Chris > default: > return false; > } > @@ -1035,7 +1038,7 @@ static void vop_plane_atomic_update(struct drm_plane > *plane, > VOP_WIN_SET(vop, win, dsp_info, dsp_info); > VOP_WIN_SET(vop, win, dsp_st, dsp_st); > > - rb_swap = has_rb_swapped(fb->format->format); > + rb_swap = has_rb_swapped(vop->data->version, fb->format->format); > VOP_WIN_SET(vop, win, rb_swap, rb_swap); > > /*
Hi: On 10/24/23 16:49, Christopher Obbard wrote: > Hi Jonas, > > On Mon, 2023-10-23 at 21:11 +0000, Jonas Karlman wrote: >> Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328 >> and RK3399 result in wrong colors being displayed. >> >> The issue can be observed using modetest: >> >> modetest -s <connector_id>@<crtc_id>:1920x1080-60@RG24 >> modetest -s <connector_id>@<crtc_id>:1920x1080-60@BG24 >> >> Vendor 4.4 kernel apply an inverted rb swap for these formats on VOP >> full (major = 3). >> >> Fix colors by applying rb swap similar to vendor 4.4 kernel. >> >> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > How about a fixes tag? Seems like this was introduced in commit 85a359f25388 > ("drm/rockchip: Add BGR formats to VOP") > >> --- >> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> index b3d0b6ae9294..a1ce09a22f83 100644 >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >> @@ -247,14 +247,17 @@ static inline void vop_cfg_done(struct vop *vop) >> VOP_REG_SET(vop, common, cfg_done, 1); >> } >> >> -static bool has_rb_swapped(uint32_t format) >> +static bool has_rb_swapped(uint32_t version, uint32_t format) >> { >> switch (format) { >> case DRM_FORMAT_XBGR8888: >> case DRM_FORMAT_ABGR8888: >> - case DRM_FORMAT_BGR888: >> case DRM_FORMAT_BGR565: >> return true; >> + case DRM_FORMAT_RGB888: >> + return VOP_MAJOR(version) == 3; >> + case DRM_FORMAT_BGR888: >> + return VOP_MAJOR(version) != 3; > The hardcoded bits are quite scary as it applies to all hardware variants ;-). > Is it worth adding an inline comment to describe what is going on and how it > relates to VOPL and VOPH? Or would it be even better to add this as a quirk in > the various vop_data structs? Every vop hardware has a version(including VOPB/VOPL), so I think use VOP_MAJOR to distinguish is ok. Of course it's better to add some comments. As for adding some quirk in vop_data, I have the idea of adding a table to describe the drm format and it's (rb/uv swap, afbc) capability, but I think this is can be done in the future. > > > Cheers! > > Chris > >> default: >> return false; >> } >> @@ -1035,7 +1038,7 @@ static void vop_plane_atomic_update(struct drm_plane >> *plane, >> VOP_WIN_SET(vop, win, dsp_info, dsp_info); >> VOP_WIN_SET(vop, win, dsp_st, dsp_st); >> >> - rb_swap = has_rb_swapped(fb->format->format); >> + rb_swap = has_rb_swapped(vop->data->version, fb->format->format); >> VOP_WIN_SET(vop, win, rb_swap, rb_swap); >> >> /* > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
On 2023-10-24 14:41, Andy Yan wrote: > Hi: > > On 10/24/23 16:49, Christopher Obbard wrote: >> Hi Jonas, >> >> On Mon, 2023-10-23 at 21:11 +0000, Jonas Karlman wrote: >>> Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328 >>> and RK3399 result in wrong colors being displayed. >>> >>> The issue can be observed using modetest: >>> >>> modetest -s <connector_id>@<crtc_id>:1920x1080-60@RG24 >>> modetest -s <connector_id>@<crtc_id>:1920x1080-60@BG24 >>> >>> Vendor 4.4 kernel apply an inverted rb swap for these formats on VOP >>> full (major = 3). >>> >>> Fix colors by applying rb swap similar to vendor 4.4 kernel. >>> >>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >> How about a fixes tag? Seems like this was introduced in commit 85a359f25388 >> ("drm/rockchip: Add BGR formats to VOP") That commit added BGR888 format, the RGB888 format goes back to from when driver was initially added. This patch depend on a macro that was introduced later, in commit eb5cb6aa9a72 ("drm/rockchip: vop: add a series of vop support"). Still think commit 85a359f25388 is best commit to use in a fixes tag. Will include a fixes tag for 85a359f25388 in v2. >> >>> --- >>> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> index b3d0b6ae9294..a1ce09a22f83 100644 >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c >>> @@ -247,14 +247,17 @@ static inline void vop_cfg_done(struct vop *vop) >>> VOP_REG_SET(vop, common, cfg_done, 1); >>> } >>> >>> -static bool has_rb_swapped(uint32_t format) >>> +static bool has_rb_swapped(uint32_t version, uint32_t format) >>> { >>> switch (format) { >>> case DRM_FORMAT_XBGR8888: >>> case DRM_FORMAT_ABGR8888: >>> - case DRM_FORMAT_BGR888: >>> case DRM_FORMAT_BGR565: >>> return true; >>> + case DRM_FORMAT_RGB888: >>> + return VOP_MAJOR(version) == 3; >>> + case DRM_FORMAT_BGR888: >>> + return VOP_MAJOR(version) != 3; >> The hardcoded bits are quite scary as it applies to all hardware variants ;-). >> Is it worth adding an inline comment to describe what is going on and how it >> relates to VOPL and VOPH? Or would it be even better to add this as a quirk in >> the various vop_data structs? > I will add a comment in v2. It would be a quirk that apply for all VOP full framework, IP version 3.x, SoCs so checking VOP_MAJOR seem like a good option. See commit eb5cb6aa9a72 ("drm/rockchip: vop: add a series of vop support") for a list of SoCs that use VOP full framework: Vop Full framework now has following vops: IP version chipname 3.1 rk3288 3.2 rk3368 3.4 rk3366 3.5 rk3399 big 3.6 rk3399 lit 3.7 rk3228 3.8 rk3328 major version: used for IP structure, Vop full framework is 3, vop little framework is 2. There are currently three struct vop_data that is missing version declaration: - rk3036_vop should use VOP_VERSION(2, 2) - rk3126_vop should use VOP_VERSION(2, 4) - rk3188_vop guessing is 2.0/2.1 (same/similar as rk3066) Since none of them are 3.x / VOP full framework, this patch should only fix/change behavior for affected 3.x / VOP full framework SoCs. Regards, Jonas > > Every vop hardware has a version(including VOPB/VOPL), so I think use > VOP_MAJOR to distinguish is ok. Of course it's better > > to add some comments. As for adding some quirk in vop_data, I have the > idea of adding a table to describe the drm format and it's (rb/uv swap, > afbc) > > capability, but I think this is can be done in the future. > >> >> >> Cheers! >> >> Chris >> >>> default: >>> return false; >>> } >>> @@ -1035,7 +1038,7 @@ static void vop_plane_atomic_update(struct drm_plane >>> *plane, >>> VOP_WIN_SET(vop, win, dsp_info, dsp_info); >>> VOP_WIN_SET(vop, win, dsp_st, dsp_st); >>> >>> - rb_swap = has_rb_swapped(fb->format->format); >>> + rb_swap = has_rb_swapped(vop->data->version, fb->format->format); >>> VOP_WIN_SET(vop, win, rb_swap, rb_swap); >>> >>> /*
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index b3d0b6ae9294..a1ce09a22f83 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -247,14 +247,17 @@ static inline void vop_cfg_done(struct vop *vop) VOP_REG_SET(vop, common, cfg_done, 1); } -static bool has_rb_swapped(uint32_t format) +static bool has_rb_swapped(uint32_t version, uint32_t format) { switch (format) { case DRM_FORMAT_XBGR8888: case DRM_FORMAT_ABGR8888: - case DRM_FORMAT_BGR888: case DRM_FORMAT_BGR565: return true; + case DRM_FORMAT_RGB888: + return VOP_MAJOR(version) == 3; + case DRM_FORMAT_BGR888: + return VOP_MAJOR(version) != 3; default: return false; } @@ -1035,7 +1038,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane, VOP_WIN_SET(vop, win, dsp_info, dsp_info); VOP_WIN_SET(vop, win, dsp_st, dsp_st); - rb_swap = has_rb_swapped(fb->format->format); + rb_swap = has_rb_swapped(vop->data->version, fb->format->format); VOP_WIN_SET(vop, win, rb_swap, rb_swap); /*