lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <d6c77064-bae5-41c3-b49f-8c5c3a076a6b@rock-chips.com> Date: Thu, 16 Nov 2023 16:00:06 +0800 From: Andy Yan <andy.yan@...k-chips.com> To: Sascha Hauer <s.hauer@...gutronix.de> Cc: Andy Yan <andyshrk@....com>, heiko@...ech.de, hjc@...k-chips.com, dri-devel@...ts.freedesktop.org, linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org, krzysztof.kozlowski+dt@...aro.org, robh+dt@...nel.org, devicetree@...r.kernel.org, sebastian.reichel@...labora.com, kever.yang@...k-chips.com, chris.obbard@...labora.com Subject: Re: [PATCH 09/11] drm/rockchip: vop2: Add support for rk3588 Hi Sascha: On 11/16/23 15:50, Sascha Hauer wrote: > On Thu, Nov 16, 2023 at 03:24:54PM +0800, Andy Yan wrote: >>> case ROCKCHIP_VOP2_EP_HDMI0: >>> case ROCKCHIP_VOP2_EP_HDMI1: >>> ... >>> } >>> >>> would look a bit better overall. >>> >>>> + /* >>>> + * K = 2: dclk_core = if_pixclk_rate > if_dclk_rate >>>> + * K = 1: dclk_core = hdmie_edp_dclk > if_pixclk_rate >>>> + */ >>>> + if (output_mode == ROCKCHIP_OUT_MODE_YUV420) { >>>> + dclk_rate = dclk_rate >> 1; >>>> + K = 2; >>>> + } >>>> + >>>> + if_pixclk_rate = (dclk_core_rate << 1) / K; >>>> + if_dclk_rate = dclk_core_rate / K; >>>> + >>>> + *if_pixclk_div = dclk_rate / if_pixclk_rate; >>>> + *if_dclk_div = dclk_rate / if_dclk_rate; >>> Not sure if this will change with future extensions, but currently >>> *if_pixclk_div will always be 2 and *if_dclk_div will alway be 4, >>> so maybe better write it like this >> >> Yes, the calculation of *if_pixclk_div is always 2 and *if_dclk_div is always 4, >> >> I think calculation formula can give us a clear explanation why is 2 or 4. >> >> considering the great power of rk3588, i think it can calculate it very easy. > Sure it can. My concern is not the CPU time it takes to do that > equation, but more the readability of the code. For me as a reader it > might be more easily acceptable that both dividers have fixed values > than it is to understand the equation. > > Your mileage may vary, I won't insist on this. Or I make it as fixed values, and leave the calculation formula as comments ? > >>> >>>> + *dclk_core_div = dclk_rate / dclk_core_rate; >>> *dclk_core_div is calculated the same way for all cases. You could pull >>> this out of the if/else. >> Okay, will do. >>>> + } else if (vop2_output_if_is_edp(id)) { >>>> + /* edp_pixclk = edp_dclk > dclk_core */ >>>> + if_pixclk_rate = v_pixclk / K; >>>> + if_dclk_rate = v_pixclk / K; >>> if_dclk_rate is unused here. >> >> It will be removed in next version. >> >>>> + dclk_rate = if_pixclk_rate * K; >>>> + *dclk_core_div = dclk_rate / dclk_core_rate; >>>> + *if_pixclk_div = dclk_rate / if_pixclk_rate; >>>> + *if_dclk_div = *if_pixclk_div; >>> Both *if_pixclk_div and *if_dclk_div will always be 1. >> Actually, they will be the value of K here, if it work at split mode(two >> >> edp connect to one VP, one show the image for left half, one for right half, >> >> a function like a dual channel mipi dsi). >> >> I know it split mode is not supported by the current mainline, but i think keep > Ok. > > Sascha > >
Powered by blists - more mailing lists