Re: [PATCH v9 00/23] drm/rockchip: RK356x VOP2 support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi:

On 4/6/22 16:13, Sascha Hauer wrote:
On Wed, Apr 06, 2022 at 10:02:59AM +0800, Andy Yan wrote:
Hi:

On 4/5/22 17:37, Sascha Hauer wrote:
On Sat, Apr 02, 2022 at 09:37:17AM +0800, Andy Yan wrote:
Hi Sacha:

On 4/1/22 20:52, Sascha Hauer wrote:
--
>From cbc03073623a7180243331ac24c3afaf9dec7522 Mon Sep 17 00:00:00 2001
From: Sascha Hauer<s.hauer@xxxxxxxxxxxxxx>
Date: Fri, 1 Apr 2022 14:48:49 +0200
Subject: [PATCH] fixup! drm: rockchip: Add VOP2 driver

---
    drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 ++++++++++++++
    1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 7dba7b9b63dc6..1421bf2f133f1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2287,6 +2287,20 @@ static int vop2_create_crtc(struct vop2 *vop2)
    			}
    		}
+		if (vop2->data->soc_id == 3566) {
+			/*
+			 * On RK3566 these windows don't have an independent
+			 * framebuffer. They share the framebuffer with smart0,
+			 * esmart0 and cluster0 respectively.
+			 */
+			switch (win->data->phys_id) {
+			case ROCKCHIP_VOP2_SMART1:
+			case ROCKCHIP_VOP2_ESMART1:
+			case ROCKCHIP_VOP2_CLUSTER1:
+				continue;
+			}
Think about this , there maybe other upcoming vop2 base soc, they may only
have

mirror window Smart1 Esmart1, or Smart1, Esmart1, Esmart2, Cluster1.

I think this should add WIN_FEATURE at the platform description file
rockchip_vop2_reg.c, then

check the FEATURE to decide whether the driver should give this window a
special treatment.

this can make one code run for different soc with different platform
description. or we should add

the same code logic for different soc again and again.
You mean like done in the downstream Kernel? Here indeed we have a
WIN_FEATURE_MIRROR flag added to the platform description. This is then
evaluated with:

static bool vop2_is_mirror_win(struct vop2_win *win)
{
          return soc_is_rk3566() && (win->feature & WIN_FEATURE_MIRROR);
}

So a flag is added and afterwards its evaluation is SoC specific. That
doesn't help at all and only obfuscates things.

Besides, experience shows that you can't predict a good abstraction for
This is not a  predict,  this is an IP feature, so it will appeared on
upcoming SOC.

We have rk3588 with 8 windows(4 Cluster + 4 Esmart, no Smart window), and

also have a entry level soc which only have 4 windows, they both have this
feature.
Same as with the other discussion: Please let's solve this once we are
there.


I am not sure if this is the suitable way for upstream, this sound like

just solve the issue appeared at the front of eyes and not think any

thing about make this driver easy to support new hardware in the future.

For now my addition is the easiest way out. Once other SoCs shall be
supported we can re-evaluate that and find better suitable ways for SoC
abstractions. This may result in just your suggestion (in which case you
can say told-you-so) or completely different.

Sascha




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux