[radeon-alex:drm-next-4.19-wip 47/115] drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c:3769:5: warning: 'result' may be used uninitialized in this function
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next-4.19-wip head: 69c20a808c86d7fd6cd64a9c8cc6b024a88c45fa commit: fcad2435f489c3510cc95f6a38ff7db2f6292b6f [47/115] drm/amd/powerplay: Set higher SCLK&MCLK frequency than dpm7 in OD config: x86_64-randconfig-s2-06141023 (attached as .config) compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026 reproduce: git checkout fcad2435f489c3510cc95f6a38ff7db2f6292b6f # save the attached .config to linux build tree make ARCH=x86_64 Note: it may well be a FALSE warning. FWIW you are at least aware of it now. http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings All warnings (new ones prefixed by >>): drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c: In function 'smu7_set_power_state_tasks': >> drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c:3769:5: warning: >> 'result' may be used uninitialized in this function [-Wmaybe-uninitialized] if (result) ^ drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c:3758:6: note: 'result' was declared here int result; ^~ vim +/result +3769 drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu7_hwmgr.c 599a7e9f Rex Zhu 2016-09-09 3754 599a7e9f Rex Zhu 2016-09-09 3755 static int smu7_generate_dpm_level_enable_mask( 599a7e9f Rex Zhu 2016-09-09 3756 struct pp_hwmgr *hwmgr, const void *input) 599a7e9f Rex Zhu 2016-09-09 3757 { 599a7e9f Rex Zhu 2016-09-09 3758 int result; 599a7e9f Rex Zhu 2016-09-09 3759 const struct phm_set_power_state_input *states = 599a7e9f Rex Zhu 2016-09-09 3760 (const struct phm_set_power_state_input *)input; 599a7e9f Rex Zhu 2016-09-09 3761 struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend); 599a7e9f Rex Zhu 2016-09-09 3762 const struct smu7_power_state *smu7_ps = 599a7e9f Rex Zhu 2016-09-09 3763 cast_const_phw_smu7_power_state(states->pnew_state); 599a7e9f Rex Zhu 2016-09-09 3764 fcad2435 Kenneth Feng 2018-06-12 3765 /*skip the trim if od is enabled*/ fcad2435 Kenneth Feng 2018-06-12 3766 if (!hwmgr->od_enabled) 599a7e9f Rex Zhu 2016-09-09 3767 result = smu7_trim_dpm_states(hwmgr, smu7_ps); fcad2435 Kenneth Feng 2018-06-12 3768 599a7e9f Rex Zhu 2016-09-09 @3769 if (result) 599a7e9f Rex Zhu 2016-09-09 3770 return result; 599a7e9f Rex Zhu 2016-09-09 3771 599a7e9f Rex Zhu 2016-09-09 3772 data->dpm_level_enable_mask.sclk_dpm_enable_mask = 599a7e9f Rex Zhu 2016-09-09 3773 phm_get_dpm_level_enable_mask_value(&data->dpm_table.sclk_table); 599a7e9f Rex Zhu 2016-09-09 3774 data->dpm_level_enable_mask.mclk_dpm_enable_mask = 599a7e9f Rex Zhu 2016-09-09 3775 phm_get_dpm_level_enable_mask_value(&data->dpm_table.mclk_table); 599a7e9f Rex Zhu 2016-09-09 3776 data->dpm_level_enable_mask.pcie_dpm_enable_mask = 599a7e9f Rex Zhu 2016-09-09 3777 phm_get_dpm_level_enable_mask_value(&data->dpm_table.pcie_speed_table); 599a7e9f Rex Zhu 2016-09-09 3778 599a7e9f Rex Zhu 2016-09-09 3779 return 0; 599a7e9f Rex Zhu 2016-09-09 3780 } 599a7e9f Rex Zhu 2016-09-09 3781 :: The code at line 3769 was first introduced by commit :: 599a7e9fe1b683d04f889d68f866f5548b1e0239 drm/amd/powerplay: implement smu7 hwmgr to manager asics with smu ip version 7. :: TO: Rex Zhu :: CC: Alex Deucher --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 200045] black screen on 'radeon' module probing
https://bugzilla.kernel.org/show_bug.cgi?id=200045 --- Comment #6 from Wolfram Sang (w...@the-dreams.de) --- So, I really posted the patches now. Sorry, I got side-tracked. They can be found here: http://patchwork.ozlabs.org/patch/929185/ http://patchwork.ozlabs.org/patch/929186/ I set you on CC as well. As indicated in the cover letter, could you try these patches one by one? Thanks and kind regards. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 200045] black screen on 'radeon' module probing
https://bugzilla.kernel.org/show_bug.cgi?id=200045 --- Comment #5 from Wolfram Sang (w...@the-dreams.de) --- Sure. But reading the original description above, I think the default use case is to not use hw_i2c. It was just added to try to work around the regression. And that led to another bug. Or am I wrong here? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:drm-next-4.19-wip 110/115] htmldocs: drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:90: warning: Function parameter or member 'type' not described in 'amdgpu_mn'
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next-4.19-wip head: 69c20a808c86d7fd6cd64a9c8cc6b024a88c45fa commit: b55f236b433d63a5c4c90c5a40c9694eb5cbff34 [110/115] drm/amdgpu: fix documentation of amdgpu_mn.c v2 reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev' include/net/mac80211.h:2282: warning: Function parameter or member 'radiotap_timestamp.units_pos' not described in 'ieee80211_hw' include/net/mac80211.h:2282: warning: Function parameter or member 'radiotap_timestamp.accuracy' not described in 'ieee80211_hw' include/net/mac80211.h:955: warning: Function parameter or member 'control.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.use_rts' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.use_cts_prot' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.short_preamble' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.skip_table' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.jiffies' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.vif' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.hw_key' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.flags' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.enqueue_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'ack' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.is_valid_ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.status_driver_data' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info' net/mac802
[radeon-alex:amd-staging-drm-next 610/619] htmldocs: drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:359: warning: Function parameter or member 'pte_support_ats' not described in 'amdgpu_vm_clear_bo'
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next head: efd7788c4a440020f2c14a080275859935759af2 commit: ad1a77f3d449b086b26428f0fc4f8ad7e7481b4a [610/619] drm/amdgpu: Update function level documentation for GPUVM v3 reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.ablkcipher' not described in 'crypto_alg' include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.blkcipher' not described in 'crypto_alg' include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.cipher' not described in 'crypto_alg' include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.compress' not described in 'crypto_alg' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.prev_bssid' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev' include/net/mac80211.h:2263: warning: Function parameter or member 'radiotap_timestamp.units_pos' not described in 'ieee80211_hw' include/net/mac80211.h:2263: warning: Function parameter or member 'radiotap_timestamp.accuracy' not described in 'ieee80211_hw' include/net/mac80211.h:950: warning: Function parameter or member 'rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.use_rts' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.use_cts_prot' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.short_preamble' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.skip_table' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.jiffies' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.vif' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.hw_key' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.flags' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.enqueue_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'ack' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status_driver_data' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: wa
[radeon-alex:amd-staging-drm-next 114/619] sound/soc/amd/raven/acp3x.h:28:9: error: implicit declaration of function 'readl'; did you mean 'vread'?
Hi Maruthi, FYI, the error/warning still remains. tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next head: efd7788c4a440020f2c14a080275859935759af2 commit: 2a6630b1095609b26a205b7c537594f3cde99c0a [114/619] ASoC: AMD: enable ACP3x drivers build config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 2a6630b1095609b26a205b7c537594f3cde99c0a # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64 All errors (new ones prefixed by >>): In file included from sound/soc/amd/raven/acp3x-pcm-dma.c:26:0: sound/soc/amd/raven/acp3x.h: In function 'rv_readl': >> sound/soc/amd/raven/acp3x.h:28:9: error: implicit declaration of function >> 'readl'; did you mean 'vread'? [-Werror=implicit-function-declaration] return readl(base_addr - ACP3x_PHY_BASE_ADDRESS); ^ vread sound/soc/amd/raven/acp3x.h: In function 'rv_writel': >> sound/soc/amd/raven/acp3x.h:33:2: error: implicit declaration of function >> 'writel'; did you mean 'vwrite'? [-Werror=implicit-function-declaration] writel(val, base_addr - ACP3x_PHY_BASE_ADDRESS); ^~ vwrite sound/soc/amd/raven/acp3x-pcm-dma.c: In function 'acp3x_audio_probe': sound/soc/amd/raven/acp3x-pcm-dma.c:638:22: error: implicit declaration of function 'devm_ioremap'; did you mean 'of_ioremap'? [-Werror=implicit-function-declaration] adata->acp3x_base = devm_ioremap(&pdev->dev, res->start, ^~~~ of_ioremap sound/soc/amd/raven/acp3x-pcm-dma.c:638:20: warning: assignment makes pointer from integer without a cast [-Wint-conversion] adata->acp3x_base = devm_ioremap(&pdev->dev, res->start, ^ cc1: some warnings being treated as errors vim +28 sound/soc/amd/raven/acp3x.h 1e29b934 Maruthi Srinivas Bayyavarapu 2017-03-27 25 1e29b934 Maruthi Srinivas Bayyavarapu 2017-03-27 26 static inline u32 rv_readl(void __iomem *base_addr) 1e29b934 Maruthi Srinivas Bayyavarapu 2017-03-27 27 { 1e29b934 Maruthi Srinivas Bayyavarapu 2017-03-27 @28return readl(base_addr - ACP3x_PHY_BASE_ADDRESS); 1e29b934 Maruthi Srinivas Bayyavarapu 2017-03-27 29 } 1e29b934 Maruthi Srinivas Bayyavarapu 2017-03-27 30 1e29b934 Maruthi Srinivas Bayyavarapu 2017-03-27 31 static inline void rv_writel(u32 val, void __iomem *base_addr) 1e29b934 Maruthi Srinivas Bayyavarapu 2017-03-27 32 { 1e29b934 Maruthi Srinivas Bayyavarapu 2017-03-27 @33writel(val, base_addr - ACP3x_PHY_BASE_ADDRESS); :: The code at line 28 was first introduced by commit :: 1e29b93483a3e02f1126c5d9b16c4436f6a53c80 ASoC: AMD: add ACP3.0 PCI driver :: TO: Maruthi Srinivas Bayyavarapu :: CC: Alex Deucher --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Getting OpenChrome DRM mainlined into Linux kernel tree
On Mon, May 28, 2018 at 6:50 PM Kevin Brace wrote: > > > Well done on getting this far. Merging this is definitely going to be > > non-trivial. Being out of tree for so long means you've ended up in a > > place that will require retracing a bunch of steps to get upstream. > > > > Just to clarify what is going on, while OpenChrome DRM (drm-openchrome > repository) has been living outside of the mainline tree since Year 2011, I > started tracking drm-next branch of Linux DRM tree very closely since > September 2017. > The current leading edge OpenChrome DRM code compiles against Linux 4.16, and > when drm-next branch is updated to Linux 4.17 rc6 or rc7 code, the > drm-next-4.18 branch (the present leading edge branch) will pull in the code. > While I have removed drm/via subdirectory from the current drm-next-4.1* > branches, I will restore it per your suggestion regarding VIA DRM. > That being said, almost all the development activities of OpenChrome DRM goes > on inside drm/openchrome subdirectory, and maybe you have some insights I am > not aware of, but I would think it is a matter of creating a subdirectory > drm/openchrome, and sticking OpenChrome DRM code there. > That's how I have developed the code from some point on and have not > encountered any side effects due to this. > > > > I'm not going to insist on atomic modesetting, but I think it would > > definitely make the driver simpler and easier for someone to review, > > and I'm open to Daniel insisting on it. I think you'd be getting close > > to clean slating most of this driver at that point, which considering > > some bits below might not be the worst idea. > > > > Okay, I will still stay with the position of wanting OpenChrome DRM to be > "grandfathered" from the universal plane and atomic mode setting > implementations for the initial mainlining, although I am open to those two > items being added to the TODO list for the future. > I think I was one of those developers asking you to switch to atomic (iirc, i encouraged you to start working on kms instead of ums). I know this is a personal project that you've been working on in your spare time (which is awesome!), so while I still encourage you to convert the driver, I don't have a problem with you doing the conversion in mainline. I think hiding under staging until the conversion is complete is a pretty reasonable compromise. Sean > > > > Other than the universal plane and atomic mode setting, I have > > > several other concerns. > > > > > > 1) James left some unfinished acceleration related code inside OpenChrome > > > DRM, but I do not plan to activate it for the initial mainlined version. > > > Do I need to remove the code? > > > > Yes. > > > > Okay, I was thinking I will have to do this, so I wanted to bring it up. > I will start working on this in a week or so. > > > > > 2) James appears to have implemented custom Libdrm ABI / API calls. I do > > > not plan to activate it for the initial mainlined version. Do I need to > > > remove the code? > > > > Yes. > > > > Same as the previous answer. > > > > > 3) Almost all the functions start with "via_" instead of "openchrome_" at > > > this point. Do I need to convert them all to "'openchrome_'?" > > > > It would be nice, but possibly not essential. > > > > I was thinking I "should" do this, but I wanted to see if the maintainer > prefers me to do this. > I will start working on this after I get the unfinished acceleration related > code removed. > > > > > 4) Is the essentially deprecated VIA DRM going to be removed from the > > > Linux kernel tree? VIA DRM is DRI1 based, and OpenChrome DRM supersedes > > > VIA DRM for obvious reasons. Since OpenChrome DRM supersedes VIA DRM, I > > > strongly support deleting VIA DRM from the Linux kernel tree. > > > > No, since it shouldn't cause any problems with this, the current via > > drm code is only enabled if userspace DRI1 stuff is around, I'm not > > even sure there's a mesa driver that can use it at all. > > > > Okay, I will leave VIA DRM alone. > If someone else wants to remove it someday, I am okay with that. > I believe the DRI1 Mesa code for VIA has been gone since Mesa 8, so nobody > really uses VIA DRM anymore other than OpenChrome DDX. > OpenChrome DDX can operate without DRI1. > That being said, I do have plans to work on updating drm/r128, drm/savage, > drm/mga, and drm/sis eventually to support at least KMS using personally > developed reusable code base. > When this happens (this has not really happened yet due to OpenChrome DRM > taking so much of my development time), the existing DRI1 code will need to > be tossed out. > Will that be okay, or will they need to be implemented in a separate > subdirectory? > > > > I'm also not sure how the VIA output bridges are wired up, but we've > > grown a lot of code for external bridges now and it might be that the > > sil164 stuff could reuse that (or not). > > > > The external video bridge interface (VIA cal
Re: [DPU PATCH 4/4] drm/msm/dpu: use private obj to track hw resources
On 2018-06-13 09:44, Jordan Crouse wrote: On Tue, Jun 12, 2018 at 06:17:47PM -0700, Jeykumar Sankaran wrote: Switch to state based resource management. This patch overhauls the resource manager and HW allocation methods by maintaining the global resource pool and allocated hw blocks in respective drm component states. Global resource manager(RM) is tracked in private object. Allocation strategy is switched from single point allocation of HW resources for the display pipeline to per component based allocation, where each drm component allocates HW blocks mapped to it's domain and tracks them in their respective state objects. Fixes resource contention due to race conditions between user space and display thread by reserving resources only in atomic check. Signed-off-by: Jeykumar Sankaran --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 210 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 59 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 223 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 4 - drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 9 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 32 +- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 86 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 19 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h| 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 805 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 149 ++-- 11 files changed, 534 insertions(+), 1070 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 426e2ad..a484c06 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -47,6 +47,8 @@ #define RIGHT_MIXER 1 #define MISR_BUFF_SIZE 256 +#define MAX_VDISPLAY_SPLIT 1080 + static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc) { @@ -142,9 +144,9 @@ static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc) crtc_state = to_dpu_crtc_state(crtc->state); lm_horiz_position = 0; - for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) { + for (lm_idx = 0; lm_idx < crtc_state->num_mixers; lm_idx++) { const struct dpu_rect *lm_roi = &crtc_state->lm_bounds[lm_idx]; - struct dpu_hw_mixer *hw_lm = dpu_crtc->mixers[lm_idx].hw_lm; + struct dpu_hw_mixer *hw_lm = crtc_state->mixers[lm_idx].hw_lm; struct dpu_hw_mixer_cfg cfg; if (dpu_kms_rect_is_null(lm_roi)) @@ -182,7 +184,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, return; } - ctl = mixer->hw_ctl; + ctl = mixer->lm_ctl; lm = mixer->hw_lm; stage_cfg = &dpu_crtc->stage_cfg; cstate = to_dpu_crtc_state(crtc->state); @@ -237,7 +239,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, format->base.pixel_format, fb ? fb->modifier : 0); /* blend config update */ - for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) { + for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) { _dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate); mixer[lm_idx].flush_mask |= flush_mask; @@ -260,7 +262,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, static void _dpu_crtc_blend_setup(struct drm_crtc *crtc) { struct dpu_crtc *dpu_crtc; - struct dpu_crtc_state *dpu_crtc_state; + struct dpu_crtc_state *cstate; struct dpu_crtc_mixer *mixer; struct dpu_hw_ctl *ctl; struct dpu_hw_mixer *lm; @@ -271,26 +273,26 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc) return; dpu_crtc = to_dpu_crtc(crtc); - dpu_crtc_state = to_dpu_crtc_state(crtc->state); - mixer = dpu_crtc->mixers; + cstate = to_dpu_crtc_state(crtc->state); + mixer = cstate->mixers; DPU_DEBUG("%s\n", dpu_crtc->name); - if (dpu_crtc->num_mixers > CRTC_DUAL_MIXERS) { - DPU_ERROR("invalid number mixers: %d\n", dpu_crtc->num_mixers); + if (cstate->num_mixers > CRTC_DUAL_MIXERS) { + DPU_ERROR("invalid number mixers: %d\n", cstate->num_mixers); Nit - this could be worded a bit better - "too many mixers" would be better, but I have to ask - under what circumstances would the number of mixers be larger than CRTC_DUAL_MIXERS and/or why don't we support a dynamic number of mixers? Comes from downstream driver implementation where CRTC iterates through RM free pool to identify mixers tagged with the current crtc id. If the previous clean up was screwed up this may have more than 2 mixers. With the new state based RM, its very unlikely we hit this condition. We do support dynamic mixer count
Re: [Freedreno] [DPU PATCH 1/4] drm/msm/dpu: add atomic private object to dpu kms
On 2018-06-13 09:29, Jordan Crouse wrote: On Tue, Jun 12, 2018 at 06:17:44PM -0700, Jeykumar Sankaran wrote: Subclass drm private state for DPU for handling driver specific data. Adds atomic private object and private object lock to dpu kms. Provides helper function to retrieve DPU private data from current atomic state. Signed-off-by: Jeykumar Sankaran --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15 2 files changed, 81 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index fe614c0..a4ab783 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1076,6 +1076,10 @@ static void dpu_kms_destroy(struct msm_kms *kms) dpu_kms = to_dpu_kms(kms); _dpu_kms_hw_destroy(dpu_kms); + + drm_atomic_private_obj_fini(&dpu_kms->priv_obj); + drm_modeset_lock_fini(&dpu_kms->priv_obj_lock); + } static void dpu_kms_preclose(struct msm_kms *kms, struct drm_file *file) @@ -1618,10 +1622,59 @@ static int dpu_kms_hw_init(struct msm_kms *kms) return rc; } +struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state *state) +{ + struct msm_drm_private *priv = state->dev->dev_private; + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); + struct drm_private_state *priv_state; + int rc = 0; + + rc = drm_modeset_lock(&dpu_kms->priv_obj_lock, state->acquire_ctx); + if (rc) + return ERR_PTR(rc); + + priv_state = drm_atomic_get_private_obj_state(state, + &dpu_kms->priv_obj); + if (!priv_state) + return NULL; I'll have to see later when this function is used, but I generally don't like it when functions return both ERR_PTR and NULL on error but I'm leaving open the possibility that this could NULL for legitimate reasons. If not, please convert to a ERR_PTR. I see drm_atomic_get_private_obj_state returns ERR_PTR(-ENOMEM) on error cases. So I should be good with IS_ERR/ERR_PTR. + return to_dpu_private_state(priv_state); +} + +static struct drm_private_state * +dpu_private_obj_duplicate_state(struct drm_private_obj *obj) +{ + struct dpu_private_state *dpu_priv_state; + + dpu_priv_state = kmemdup(obj->state, + sizeof(*dpu_priv_state), GFP_KERNEL); + if (!dpu_priv_state) + return NULL; + + __drm_atomic_helper_private_obj_duplicate_state(obj, + &dpu_priv_state->base); + + return &dpu_priv_state->base; +} + +static void dpu_private_obj_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + struct dpu_private_state *dpu_priv_state = to_dpu_private_state(state); + + kfree(dpu_priv_state); +} + +static const struct drm_private_state_funcs priv_obj_funcs = { + .atomic_duplicate_state = dpu_private_obj_duplicate_state, + .atomic_destroy_state = dpu_private_obj_destroy_state, +}; + struct msm_kms *dpu_kms_init(struct drm_device *dev) { struct msm_drm_private *priv; struct dpu_kms *dpu_kms; + struct dpu_private_state *dpu_priv_state; int irq; if (!dev || !dev->dev_private) { @@ -1639,6 +1692,19 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev) } dpu_kms->base.irq = irq; + /* Initialize private obj's */ + drm_modeset_lock_init(&dpu_kms->priv_obj_lock); + + dpu_priv_state = kzalloc(sizeof(*dpu_priv_state), GFP_KERNEL); + if (!dpu_priv_state) { + DPU_ERROR("failed to allocate dpu priv obj\n"); We don't need an error message on memory failure - you will have no problem identifying when this went boom if it goes boom. Will remove + return ERR_PTR(-ENOMEM); + } + + drm_atomic_private_obj_init(&dpu_kms->priv_obj, + &dpu_priv_state->base, + &priv_obj_funcs); + return &dpu_kms->base; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 046e6f7..924d8967 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -190,6 +190,9 @@ struct dpu_kms { struct dpu_hw_vbif *hw_vbif[VBIF_MAX]; struct dpu_hw_mdp *hw_mdp; + struct drm_modeset_lock priv_obj_lock; + struct drm_private_obj priv_obj; + bool has_danger_ctrl; struct platform_device *pdev; @@ -197,12 +200,24 @@ struct dpu_kms { struct dss_module_power mp; }; +struct dpu_private_state { + struct drm_private_state base; +}; + struct vsync_info { u32 frame_count; u32 line_count; }; #define to_dpu_kms(x) container_of(x, struct dpu_kms, base) +#define to_dpu_private_state(x) container_
Re: [PATCH 2/2] drm/doc: Make naming consistent for Core Driver Infrastructure
On Mon, Jun 4, 2018 at 5:11 AM, Michel Dänzer wrote: > > Adding dri-devel. > Any opinions? Alex > > On 2018-06-01 08:03 PM, Alex Deucher wrote: >> Use chapter rather than section to align with the rst markup. >> >> Signed-off-by: Alex Deucher >> --- >> Documentation/gpu/amdgpu.rst | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/Documentation/gpu/amdgpu.rst b/Documentation/gpu/amdgpu.rst >> index 1d726b90a619..e99732553c71 100644 >> --- a/Documentation/gpu/amdgpu.rst >> +++ b/Documentation/gpu/amdgpu.rst >> @@ -8,7 +8,7 @@ Next (GCN) architecture. >> Core Driver Infrastructure >> == >> >> -This section covers core driver infrastructure. >> +This chapter covers core driver infrastructure. >> >> PRIME Buffer Sharing >> > > I don't mind either way, but I copied the "section" wording from i915.rst. > > > -- > Earthling Michel Dänzer | http://www.amd.com > Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Patch "x86/cpufeature: Remove cpu_has_clflush" has been added to the 4.4-stable tree
This is a note to let you know that I've just added the patch titled x86/cpufeature: Remove cpu_has_clflush to the 4.4-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: x86-cpufeature-remove-cpu_has_clflush.patch and it can be found in the queue-4.4 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let know about it. From 906bf7fda2c9cf5c1762ec607943ed54b6c5b203 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Tue, 29 Mar 2016 17:41:59 +0200 Subject: x86/cpufeature: Remove cpu_has_clflush From: Borislav Petkov commit 906bf7fda2c9cf5c1762ec607943ed54b6c5b203 upstream. Use the fast variant in the DRM code. Signed-off-by: Borislav Petkov Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: dri-devel@lists.freedesktop.org Cc: intel-...@lists.freedesktop.org Link: http://lkml.kernel.org/r/1459266123-21878-7-git-send-email...@alien8.de Signed-off-by: Ingo Molnar Signed-off-by: Greg Kroah-Hartman --- arch/x86/include/asm/cpufeature.h |1 - arch/x86/kernel/cpu/intel.c|2 +- arch/x86/kernel/tce_64.c |2 +- arch/x86/mm/pageattr.c |2 +- drivers/gpu/drm/drm_cache.c|6 +++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +- 6 files changed, 7 insertions(+), 8 deletions(-) --- a/arch/x86/include/asm/cpufeature.h +++ b/arch/x86/include/asm/cpufeature.h @@ -378,7 +378,6 @@ extern const char * const x86_bug_flags[ #define cpu_has_aesboot_cpu_has(X86_FEATURE_AES) #define cpu_has_avxboot_cpu_has(X86_FEATURE_AVX) #define cpu_has_avx2 boot_cpu_has(X86_FEATURE_AVX2) -#define cpu_has_clflushboot_cpu_has(X86_FEATURE_CLFLUSH) #define cpu_has_xsave boot_cpu_has(X86_FEATURE_XSAVE) #define cpu_has_xsaves boot_cpu_has(X86_FEATURE_XSAVES) #define cpu_has_hypervisor boot_cpu_has(X86_FEATURE_HYPERVISOR) --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -455,7 +455,7 @@ static void init_intel(struct cpuinfo_x8 set_cpu_cap(c, X86_FEATURE_PEBS); } - if (c->x86 == 6 && cpu_has_clflush && + if (c->x86 == 6 && boot_cpu_has(X86_FEATURE_CLFLUSH) && (c->x86_model == 29 || c->x86_model == 46 || c->x86_model == 47)) set_cpu_bug(c, X86_BUG_CLFLUSH_MONITOR); --- a/arch/x86/kernel/tce_64.c +++ b/arch/x86/kernel/tce_64.c @@ -40,7 +40,7 @@ static inline void flush_tce(void* tceaddr) { /* a single tce can't cross a cache line */ - if (cpu_has_clflush) + if (boot_cpu_has(X86_FEATURE_CLFLUSH)) clflush(tceaddr); else wbinvd(); --- a/arch/x86/mm/pageattr.c +++ b/arch/x86/mm/pageattr.c @@ -1481,7 +1481,7 @@ static int change_page_attr_set_clr(unsi * error case we fall back to cpa_flush_all (which uses * WBINVD): */ - if (!ret && cpu_has_clflush) { + if (!ret && boot_cpu_has(X86_FEATURE_CLFLUSH)) { if (cpa.flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) { cpa_flush_array(addr, numpages, cache, cpa.flags, pages); --- a/drivers/gpu/drm/drm_cache.c +++ b/drivers/gpu/drm/drm_cache.c @@ -72,7 +72,7 @@ drm_clflush_pages(struct page *pages[], { #if defined(CONFIG_X86) - if (cpu_has_clflush) { + if (static_cpu_has(X86_FEATURE_CLFLUSH)) { drm_cache_flush_clflush(pages, num_pages); return; } @@ -105,7 +105,7 @@ void drm_clflush_sg(struct sg_table *st) { #if defined(CONFIG_X86) - if (cpu_has_clflush) { + if (static_cpu_has(X86_FEATURE_CLFLUSH)) { struct sg_page_iter sg_iter; mb(); @@ -129,7 +129,7 @@ void drm_clflush_virt_range(void *addr, unsigned long length) { #if defined(CONFIG_X86) - if (cpu_has_clflush) { + if (static_cpu_has(X86_FEATURE_CLFLUSH)) { const int size = boot_cpu_data.x86_clflush_size; void *end = addr + length; addr = (void *)(((unsigned long)addr) & -size); --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -466,7 +466,7 @@ i915_gem_execbuffer_relocate_entry(struc ret = relocate_entry_cpu(obj, reloc, target_offset); else if (obj->map_and_fenceable) ret = relocate_entry_gtt(obj, reloc, target_offset); - else if (cpu_has_clflush) + else if (static_cpu_has(X86_FEATURE_CLFLUSH)) ret = relocate_entry_clflush(obj, reloc, target_offset); else { WARN_ONCE(1, "Impossible case in relocation handling\n"); Patches currently in stable-queue which might be from b...@suse.de are queue-4.4/x86-fpu-disable-mpx-when-eagerfpu-is-off.pa
Re: [DPU PATCH 4/4] drm/msm/dpu: use private obj to track hw resources
On Tue, Jun 12, 2018 at 06:17:47PM -0700, Jeykumar Sankaran wrote: > Switch to state based resource management. This patch > overhauls the resource manager and HW allocation methods by > maintaining the global resource pool and allocated hw > blocks in respective drm component states. > > Global resource manager(RM) is tracked in private object. > Allocation strategy is switched from single point allocation > of HW resources for the display pipeline to per component > based allocation, where each drm component allocates HW > blocks mapped to it's domain and tracks them in their respective > state objects. > > Fixes resource contention due to race conditions between > user space and display thread by reserving resources > only in atomic check. > > Signed-off-by: Jeykumar Sankaran > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 210 +++--- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 59 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 223 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h| 4 - > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 9 +- > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 32 +- > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 86 +-- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 19 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h| 8 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 805 > ++--- > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 149 ++-- > 11 files changed, 534 insertions(+), 1070 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 426e2ad..a484c06 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -47,6 +47,8 @@ > #define RIGHT_MIXER 1 > > #define MISR_BUFF_SIZE 256 > +#define MAX_VDISPLAY_SPLIT 1080 > + > > static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc) > { > @@ -142,9 +144,9 @@ static void _dpu_crtc_program_lm_output_roi(struct > drm_crtc *crtc) > crtc_state = to_dpu_crtc_state(crtc->state); > > lm_horiz_position = 0; > - for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) { > + for (lm_idx = 0; lm_idx < crtc_state->num_mixers; lm_idx++) { > const struct dpu_rect *lm_roi = &crtc_state->lm_bounds[lm_idx]; > - struct dpu_hw_mixer *hw_lm = dpu_crtc->mixers[lm_idx].hw_lm; > + struct dpu_hw_mixer *hw_lm = crtc_state->mixers[lm_idx].hw_lm; > struct dpu_hw_mixer_cfg cfg; > > if (dpu_kms_rect_is_null(lm_roi)) > @@ -182,7 +184,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc > *crtc, > return; > } > > - ctl = mixer->hw_ctl; > + ctl = mixer->lm_ctl; > lm = mixer->hw_lm; > stage_cfg = &dpu_crtc->stage_cfg; > cstate = to_dpu_crtc_state(crtc->state); > @@ -237,7 +239,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc > *crtc, > format->base.pixel_format, fb ? fb->modifier : 0); > > /* blend config update */ > - for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) { > + for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) { > _dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate); > > mixer[lm_idx].flush_mask |= flush_mask; > @@ -260,7 +262,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc > *crtc, > static void _dpu_crtc_blend_setup(struct drm_crtc *crtc) > { > struct dpu_crtc *dpu_crtc; > - struct dpu_crtc_state *dpu_crtc_state; > + struct dpu_crtc_state *cstate; > struct dpu_crtc_mixer *mixer; > struct dpu_hw_ctl *ctl; > struct dpu_hw_mixer *lm; > @@ -271,26 +273,26 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc) > return; > > dpu_crtc = to_dpu_crtc(crtc); > - dpu_crtc_state = to_dpu_crtc_state(crtc->state); > - mixer = dpu_crtc->mixers; > + cstate = to_dpu_crtc_state(crtc->state); > + mixer = cstate->mixers; > > DPU_DEBUG("%s\n", dpu_crtc->name); > > - if (dpu_crtc->num_mixers > CRTC_DUAL_MIXERS) { > - DPU_ERROR("invalid number mixers: %d\n", dpu_crtc->num_mixers); > + if (cstate->num_mixers > CRTC_DUAL_MIXERS) { > + DPU_ERROR("invalid number mixers: %d\n", cstate->num_mixers); Nit - this could be worded a bit better - "too many mixers" would be better, but I have to ask - under what circumstances would the number of mixers be larger than CRTC_DUAL_MIXERS and/or why don't we support a dynamic number of mixers? > return; > } > +static void _dpu_crtc_setup_mixers(struct drm_crtc_state *crtc_state) > { > - struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); > - struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); > - st
Re: [Freedreno] [DPU PATCH 1/4] drm/msm/dpu: add atomic private object to dpu kms
On Tue, Jun 12, 2018 at 06:17:44PM -0700, Jeykumar Sankaran wrote: > Subclass drm private state for DPU for handling driver > specific data. Adds atomic private object and private object > lock to dpu kms. Provides helper function to retrieve DPU > private data from current atomic state. > > Signed-off-by: Jeykumar Sankaran > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66 > + > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15 > 2 files changed, 81 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index fe614c0..a4ab783 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -1076,6 +1076,10 @@ static void dpu_kms_destroy(struct msm_kms *kms) > > dpu_kms = to_dpu_kms(kms); > _dpu_kms_hw_destroy(dpu_kms); > + > + drm_atomic_private_obj_fini(&dpu_kms->priv_obj); > + drm_modeset_lock_fini(&dpu_kms->priv_obj_lock); > + > } > > static void dpu_kms_preclose(struct msm_kms *kms, struct drm_file *file) > @@ -1618,10 +1622,59 @@ static int dpu_kms_hw_init(struct msm_kms *kms) > return rc; > } > > +struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state > *state) > +{ > + struct msm_drm_private *priv = state->dev->dev_private; > + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > + struct drm_private_state *priv_state; > + int rc = 0; > + > + rc = drm_modeset_lock(&dpu_kms->priv_obj_lock, state->acquire_ctx); > + if (rc) > + return ERR_PTR(rc); > + > + priv_state = drm_atomic_get_private_obj_state(state, > + &dpu_kms->priv_obj); > + if (!priv_state) > + return NULL; I'll have to see later when this function is used, but I generally don't like it when functions return both ERR_PTR and NULL on error but I'm leaving open the possibility that this could NULL for legitimate reasons. If not, please convert to a ERR_PTR. > + return to_dpu_private_state(priv_state); > +} > + > +static struct drm_private_state * > +dpu_private_obj_duplicate_state(struct drm_private_obj *obj) > +{ > + struct dpu_private_state *dpu_priv_state; > + > + dpu_priv_state = kmemdup(obj->state, > + sizeof(*dpu_priv_state), GFP_KERNEL); > + if (!dpu_priv_state) > + return NULL; > + > + __drm_atomic_helper_private_obj_duplicate_state(obj, > + &dpu_priv_state->base); > + > + return &dpu_priv_state->base; > +} > + > +static void dpu_private_obj_destroy_state(struct drm_private_obj *obj, > + struct drm_private_state *state) > +{ > + struct dpu_private_state *dpu_priv_state = to_dpu_private_state(state); > + > + kfree(dpu_priv_state); > +} > + > +static const struct drm_private_state_funcs priv_obj_funcs = { > + .atomic_duplicate_state = dpu_private_obj_duplicate_state, > + .atomic_destroy_state = dpu_private_obj_destroy_state, > +}; > + > struct msm_kms *dpu_kms_init(struct drm_device *dev) > { > struct msm_drm_private *priv; > struct dpu_kms *dpu_kms; > + struct dpu_private_state *dpu_priv_state; > int irq; > > if (!dev || !dev->dev_private) { > @@ -1639,6 +1692,19 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev) > } > dpu_kms->base.irq = irq; > > + /* Initialize private obj's */ > + drm_modeset_lock_init(&dpu_kms->priv_obj_lock); > + > + dpu_priv_state = kzalloc(sizeof(*dpu_priv_state), GFP_KERNEL); > + if (!dpu_priv_state) { > + DPU_ERROR("failed to allocate dpu priv obj\n"); We don't need an error message on memory failure - you will have no problem identifying when this went boom if it goes boom. > + return ERR_PTR(-ENOMEM); > + } > + > + drm_atomic_private_obj_init(&dpu_kms->priv_obj, > + &dpu_priv_state->base, > + &priv_obj_funcs); > + > return &dpu_kms->base; > } > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > index 046e6f7..924d8967 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > @@ -190,6 +190,9 @@ struct dpu_kms { > struct dpu_hw_vbif *hw_vbif[VBIF_MAX]; > struct dpu_hw_mdp *hw_mdp; > > + struct drm_modeset_lock priv_obj_lock; > + struct drm_private_obj priv_obj; > + > bool has_danger_ctrl; > > struct platform_device *pdev; > @@ -197,12 +200,24 @@ struct dpu_kms { > struct dss_module_power mp; > }; > > +struct dpu_private_state { > + struct drm_private_state base; > +}; > + > struct vsync_info { > u32 frame_count; > u32 line_count; > }; > > #define to_dpu_kms(x) container_of(x, struct dpu_kms, base) > +#define to_dpu_private_state(x) container_of(x, struct dpu_private_state, > ba
Re: [PATCH V2 2/2] efi/fb: Convert PCI bus address to resource if translated by the bridge
On Wednesday, June 13, 2018 05:45:48 PM Ard Biesheuvel wrote: > On 18 May 2018 at 16:17, Sinan Kaya wrote: > > A host bridge is allowed to remap BAR addresses using _TRA attribute in > > _CRS windows. > > > > pci_bus :00: root bus resource [mem 0x8010010-0x8011fff window] > > (bus address [0x0010-0x1fff]) > > pci :02:00.0: reg 0x10: [mem 0x8011e00-0x8011eff] > > > > When a VGA device is behind such a host bridge and the resource is > > translated efifb driver is trying to do ioremap against bus address > > rather than the resource address and is failing to probe. > > > > efifb: probing for efifb > > efifb: cannot reserve video memory at 0x1e00 > > efifb: framebuffer at 0x1e00, using 1920k, total 1875k > > efifb: mode is 800x600x32, linelength=3200, pages=1 > > efifb: scrolling: redraw > > efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 > > > > Use the host bridge offset information to convert bus address to > > resource address in the fixup. > > > > Signed-off-by: Sinan Kaya > > Reviewed-by: Ard Biesheuvel > > Bartlomiej, could you please take these via the fbdev tree for v4.19? Sure, I will queue it after the current merge window. > Peter already gave his ack but Sinan dropped it (presumably because of > the split in v2) Peter, can I (re)add your ACK to V2 patches? Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 106666] amdgpu 0000:09:00.0: [gfxhub] VMC page fault (src_id:0 ring:56 vmid:3 pas_id:0), [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, last signaled seq=327845, last emitted seq=32
https://bugs.freedesktop.org/show_bug.cgi?id=10 --- Comment #18 from udo --- I think that, with two times 3 days of amdgpu-problem free uptime, we can say that updates firmwares do fix this issue. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RESEND] backlight: gpio-backlight: Correct initial power state handling
On 08/05/18 08:04, Peter Ujfalusi wrote: The default-on property - or the def_value via legacy pdata) should be handled as: if it is 1, the backlight must be enabled (kept enabled) if it is 0, the backlight must be disabled (kept disabled) This only works for the case when default-on is set. If it is not set then the brightness of the backlight is set to 0. Now if the backlight is enabled by external driver (graphics) the backlight will stay disabled since the brightness is configured as 0. The backlight will not turn on. The correct action at probe time is to configure the props.power to FB_BLANK_UNBLANK if we want the backlight on by default or to FB_BLANK_POWERDOWN if the backlight should be off by default. The initial brightness should be set to 1. Hmnn... I guess this comes down to the definition of "on" for a binary Actually I'm a little worried that backlight already has too many different behaviors and that this patch introduces another way for them to be different! Is there any mileage in adopting the same approach as PWM backlight for blank/unblank management as a way to get a flicker free boot? For PWM the default property controls the initial brightness and the initial power state is unblanked *unless* there is a phandle link to the node, in which case we inherit whatever the power state the bootloader had configured before the driver probed. Put another way, what happens is we implement gpio_backlight_initial_power_state() to perform a similar task to pwm_backlight_initial_power_state(). Daniel. Signed-off-by: Peter Ujfalusi --- Hi, for some reason the original patch got lost: https://patchwork.kernel.org/patch/9445539/ But it is still valid, so I'm resending it. Regards, Peter drivers/video/backlight/gpio_backlight.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index e470da95d806..904a4462cefe 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -142,7 +142,9 @@ static int gpio_backlight_probe(struct platform_device *pdev) return PTR_ERR(bl); } - bl->props.brightness = gbl->def_value; + bl->props.power = gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN; + bl->props.brightness = 1; + backlight_update_status(bl); platform_set_drvdata(pdev, bl); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
On Wed, Jun 13, 2018 at 5:08 AM, Sandeep Panda wrote: > Document the bindings used for the sn65dsi86 DSI to eDP bridge. > > Changes in v1: > - Rephrase the dt-binding descriptions to be more inline with existing >bindings (Andrzej Hajda). > - Add missing dt-binding that are parsed by corresponding driver >(Andrzej Hajda). > > Changes in v2: > - Remove edp panel specific dt-binding entries. Only keep bridge >specific entries (Sean Paul). > - Remove custom-modes dt entry since its usage is removed from driver also > (Sean Paul). > - Remove is-pluggable dt entry since this will not be needed anymore (Sean > Paul). > > Changes in v3: > - Remove irq-gpio dt entry and instead populate is an interrupt >property (Rob Herring). > > Changes in v4: > - Add link to bridge chip datasheet (Stephen Boyd) > - Add vpll and vcc regulator supply bindings (Stephen Boyd) > - Add ref clk optional dt binding (Stephen Boyd) > - Add gpio-controller optional dt binding (Stephen Boyd) > > Changes in v5: > - Use clock property to specify the input refclk (Stephen Boyd). > - Update gpio cell and pwm cell numbers (Stephen Boyd). > > Changes in v6: > - Add property to mention the lane mapping scheme and polarity inversion >(Stephen Boyd). > > Changes in v7: > - Detail description of lane mapping scheme dt property (Andrzej >Hajda/ Rob Herring). > - Removed HDP gpio binding, since the bridge uses IRQ signal to >determine HPD, and IRQ property is already documented in binding. > > Changes in v8: > - Removed unnecessary explanation of lane mapping and polarity dt >property, since these are already explained in media/video-interface >dt binidng (Rob Herring). > > Signed-off-by: Sandeep Panda > --- > .../bindings/display/bridge/ti,sn65dsi86.txt | 90 > ++ > 1 file changed, 90 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt > > diff --git > a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt > b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt > new file mode 100644 > index 000..601454c > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt > @@ -0,0 +1,90 @@ > +SN65DSI86 DSI to eDP bridge chip > + > + > +This is the binding for Texas Instruments SN65DSI86 bridge. > +http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86&fileType=pdf > + > +Required properties: > +- compatible: Must be "ti,sn65dsi86" > +- reg: i2c address of the chip, 0x2d as per datasheet > +- enable-gpios: OF device-tree gpio specification for bridge_en pin (active > high) > + > +- vccio-supply: A 1.8V supply that powers up the digital IOs. > +- vpll-supply: A 1.8V supply that powers up the displayport PLL. > +- vcca-supply: A 1.2V supply that powers up the analog circuits. > +- vcc-supply: A 1.2V supply that powers up the digital core. > + > +Optional properties: > +- interrupts: Specifier for the SN65DSI86 interrupt line. > + > +- ddc-i2c-bus: phandle of the I2C controller used for DDC EDID probing > + > +- gpio-controller: Marks the device has a GPIO controller. > +- #gpio-cells: Should be two. The first cell is the pin number and > + the second cell is used to specify flags. > + See ../../gpio/gpio.txt for more information. > +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of > + the cell formats. > + > +- clock-names: should be "refclk" > +- clocks: Specification for input reference clock. The reference > + clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz. > + > +- data-lanes: Specification to describe the logical to physical lane > + mapping scheme. See ../../media/video-interface.txt for more > + information. > +- lane-polarities: Specification to describe the polarity of physical lanes. > + See ../../media/video-interface.txt for more information. You are still defining the properties here. All you need is: data-lanes: See ../../media/video-interface.txt Perhaps you need to say should be 4 lanes, but OTOH everything tends to be 4 lanes. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/bridge/sii8620: simplify hardware reset procedure
On 13.06.2018 15:08, Maciej Purski wrote: > Hi Andrzej, > > On 06/08/2018 08:04 AM, Andrzej Hajda wrote: >> There is no need to flip reset pin twice. Also delays can be changed to >> values present in vendor's code. >> >> Signed-off-by: Andrzej Hajda > Reviewed-by: Maciej Purski Finally, thanks for the review. Merged to drm-misc-fixes this and other dependand fixes: drm/bridge/sii8620: simplify hardware reset procedure drm/bridge/sii8620: fix loops in EDID fetch logic drm/bridge/sii8620: fix display modes validation drm/bridge/sii8620: fix potential buffer overflow drm/bridge/sii8620: start MHL transmission after HDMI signal detection drm/bridge/sii8620: remove HSIC initialization drm/bridge/sii8620: fix HDMI cable connection to dongle Regards Andrzej > >> --- >> Hi, >> >> This is v2 of forgotten patch, awaiting reviewers, any volunteers. >> Also "drm/bridge/sii8620: fix loops in EDID fetch logic" waits for reviewers. >> >> In this version I have completely removed reset function, and moved its body >> to sii8620_hw_on. >> >> Regards >> Andrzej >> --- >> drivers/gpu/drm/bridge/sil-sii8620.c | 23 ++- >> 1 file changed, 10 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c >> b/drivers/gpu/drm/bridge/sil-sii8620.c >> index 7ab36042a822..d1e780fba4b6 100644 >> --- a/drivers/gpu/drm/bridge/sil-sii8620.c >> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c >> @@ -971,8 +971,17 @@ static int sii8620_hw_on(struct sii8620 *ctx) >> ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); >> if (ret) >> return ret; >> + >> usleep_range(1, 2); >> -return clk_prepare_enable(ctx->clk_xtal); >> +ret = clk_prepare_enable(ctx->clk_xtal); >> +if (ret) >> +return ret; >> + >> +msleep(100); >> +gpiod_set_value(ctx->gpio_reset, 0); >> +msleep(100); >> + >> +return 0; >> } >> >> static int sii8620_hw_off(struct sii8620 *ctx) >> @@ -982,17 +991,6 @@ static int sii8620_hw_off(struct sii8620 *ctx) >> return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); >> } >> >> -static void sii8620_hw_reset(struct sii8620 *ctx) >> -{ >> -usleep_range(1, 2); >> -gpiod_set_value(ctx->gpio_reset, 0); >> -usleep_range(5000, 2); >> -gpiod_set_value(ctx->gpio_reset, 1); >> -usleep_range(1, 2); >> -gpiod_set_value(ctx->gpio_reset, 0); >> -msleep(300); >> -} >> - >> static void sii8620_cbus_reset(struct sii8620 *ctx) >> { >> sii8620_write(ctx, REG_PWD_SRST, BIT_PWD_SRST_CBUS_RST >> @@ -2112,7 +2110,6 @@ static void sii8620_cable_in(struct sii8620 *ctx) >> dev_err(dev, "Error powering on, %d.\n", ret); >> return; >> } >> -sii8620_hw_reset(ctx); >> >> sii8620_read_buf(ctx, REG_VND_IDL, ver, ARRAY_SIZE(ver)); >> ret = sii8620_clear_error(ctx); >> > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 200045] black screen on 'radeon' module probing
https://bugzilla.kernel.org/show_bug.cgi?id=200045 --- Comment #4 from Alex Deucher (alexdeuc...@gmail.com) --- hw_i2c doesn't use bit banging, it uses the hw i2c engine. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] locking: Implement an algorithm choice for Wound-Wait mutexes
On 06/13/2018 03:10 PM, Peter Zijlstra wrote: On Wed, Jun 13, 2018 at 12:40:29PM +0200, Thomas Hellstrom wrote: On 06/13/2018 11:50 AM, Peter Zijlstra wrote: + + lockdep_assert_held(&lock->wait_lock); + + if (owner && hold_ctx && __ww_ctx_stamp_after(hold_ctx, ww_ctx) && + ww_ctx->acquired > 0) { + WRITE_ONCE(hold_ctx->wounded, true); + if (owner != current) { + /* +* wake_up_process() inserts a write memory barrier to It does no such thing. But yes, it does ensure the wakee sees all prior stores IFF the wakeup happened. +* make sure owner sees it is wounded before +* TASK_RUNNING in case it's sleeping on another +* ww_mutex. Note that owner points to a valid +* task_struct as long as we hold the wait_lock. +*/ What exactly are you trying to say here ? I'm thinking this is the pairing barrier to the smp_mb() below, with your list_empty() thing? Might make sense to write a single coherent comment and refer to the other location. So what I'm trying to say here is that wake_up_process() ensures that the owner, if in !TASK_RUNNING, sees the write to hold_ctx->wounded before the transition to TASK_RUNNING. This was how I interpreted "woken up" in the wake up process documentation. There is documentation!? :-) Aaah, you mean that kerneldoc comment with wake_up_process() ? Yeah, that needs fixing. /me puts on endless todo list. Anyway, wakeup providing that ordering isn't something that needs a comment of that size; and I think the only comment here is that we care about the ordering and a reference to the site(s) that pairs with it. Maybe something like: /* * __ww_mutex_lock_check_stamp() will observe our wounded store. */ Yes. Actually, I just found the set_current_state() kerneldoc which explains the built-in barrier pairing with wake_up_xxx. Perhaps I also should mention that as well. Looks like the use WRITE_ONCE() and READ_ONCE() can be dropped as well. - if (likely(!(atomic_long_read(&lock->base.owner) & MUTEX_FLAG_WAITERS))) + if (likely(list_empty(&lock->base.wait_list))) return; /* @@ -653,6 +695,17 @@ __ww_mutex_lock_check_stamp(struct mutex *lock, struct mutex_waiter *waiter, struct ww_acquire_ctx *hold_ctx = READ_ONCE(ww->ctx); struct mutex_waiter *cur; + /* +* If we miss a wounded == true here, we will have a pending Explain how we can miss that. This is actually the pairing location of the wake_up_process() comment / code discussed above. Here we should have !TASK_RUNNING, and let's say ctx->wounded is set by another process immediately after we've read it (we "miss" it). At that point there must be a pending wake-up-process() for us and we'll pick up the set value of wounded on the next iteration after returning from schedule(). Right, so that's when the above wakeup isn't the one waking us. I can't say I'm a fan. I'm already cursing the ww_mutex stuff every time I have to look at it, and you just made it worse spagethi. Well, I can't speak for the current ww implementation except I didn't think it was too hard to understand for a first time reader. Admittedly the Wound-Wait path makes it worse since it's a preemptive algorithm and we need to touch other processes a acquire contexts and worry about ordering. So, assuming your review comments are fixed up, is that a solid NAK or do you have any suggestion that would make you more comfortable with the code? like splitting out ww-stuff to a separate file? Nah, not a NAK, but we should look at whan can be done to improve code. Maybe add a few more comments that explain why. Part of the problem with ww_mutex is always that I forget exactly how they work and mutex.c doesn't have much useful comments in (most of those are in ww_mutex.h and I always forget to look there). Understood. Also; I'm not at all sure about the exact difference between what we have and what you propose. I did read the documentation part (I really should not have to) but it just doesn't jive. I suspect you're using preemption entirely different from what we usually call a preemption. I think that perhaps requires a good understanding of the difference of the algorithms in question before looking at the implementation. I put a short explanation and some URLs to CS websites describing the two algorithms and their pros and cons in the patch series introductory message. I'll forward that. In short, with Wait-Die (before the patch) it's the process _taking_ the contended lock that backs off if necessary. No preemption required. With Wound-Wait, it's the process _holding_ the contended lock that gets wounded (preempted), and it needs to back off at its own discretion but no later than when it's going to sleep on another ww mutex. That
[Bug 198713] AMD DC crashes when computing clocks/detecting freesync
https://bugzilla.kernel.org/show_bug.cgi?id=198713 --- Comment #9 from Jon (j...@moozaad.co.uk) --- Created attachment 276531 --> https://bugzilla.kernel.org/attachment.cgi?id=276531&action=edit dmesg for kernel 4.17 showing warnings with traces -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 198713] AMD DC crashes when computing clocks/detecting freesync
https://bugzilla.kernel.org/show_bug.cgi?id=198713 --- Comment #8 from Jon (j...@moozaad.co.uk) --- Updated to 4.17 as per (indirect) request :) Linux mudkip.farm 4.17.1-6-default #1 SMP PREEMPT Tue Jun 12 09:55:31 UTC 2018 (e721478) x86_64 x86_64 x86_64 GNU/Linux -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mxsfb: rename driver to mxsfb-drm
On Wed, Jun 13, 2018 at 9:38 AM, Stefan Agner wrote: > It seems to me a rather extreme measure though, given we could fix the > situation rather easily. There are dtb's using the fbdev mxsfb driver like for example: arch/arm/boot/dts/imx28-evk.dts If we kill the fbdev mxsfb driver then the display will stop working for some users and I don't think they will be happy. Last time I tried it was not possible to use the original fbdev mxsfb bindings with the new drm driver. Of course we can convert to the drm mxsfb driver, but IMHO we should avoid functional breakage all of a sudden. In the meantime Stefan's patch seems to be a good approach. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 198713] AMD DC crashes when computing clocks/detecting freesync
https://bugzilla.kernel.org/show_bug.cgi?id=198713 --- Comment #7 from Andrey Grodzovsky (andrey.grodzov...@amd.com) --- Well, the dmesg attached is also from 4.15 kernel so I assumed it's all 4.15. Anyway, I will Harry from our Display team to take a look at this. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] locking: Implement an algorithm choice for Wound-Wait mutexes
On Wed, Jun 13, 2018 at 12:40:29PM +0200, Thomas Hellstrom wrote: > On 06/13/2018 11:50 AM, Peter Zijlstra wrote: > > > > > + > > > + lockdep_assert_held(&lock->wait_lock); > > > + > > > + if (owner && hold_ctx && __ww_ctx_stamp_after(hold_ctx, ww_ctx) && > > > + ww_ctx->acquired > 0) { > > > + WRITE_ONCE(hold_ctx->wounded, true); > > > + if (owner != current) { > > > + /* > > > + * wake_up_process() inserts a write memory barrier to > > It does no such thing. But yes, it does ensure the wakee sees all prior > > stores IFF the wakeup happened. > > > > > + * make sure owner sees it is wounded before > > > + * TASK_RUNNING in case it's sleeping on another > > > + * ww_mutex. Note that owner points to a valid > > > + * task_struct as long as we hold the wait_lock. > > > + */ > > What exactly are you trying to say here ? > > > > I'm thinking this is the pairing barrier to the smp_mb() below, with > > your list_empty() thing? Might make sense to write a single coherent > > comment and refer to the other location. > > So what I'm trying to say here is that wake_up_process() ensures that the > owner, if in !TASK_RUNNING, sees the write to hold_ctx->wounded before the > transition to TASK_RUNNING. This was how I interpreted "woken up" in the > wake up process documentation. There is documentation!? :-) Aaah, you mean that kerneldoc comment with wake_up_process() ? Yeah, that needs fixing. /me puts on endless todo list. Anyway, wakeup providing that ordering isn't something that needs a comment of that size; and I think the only comment here is that we care about the ordering and a reference to the site(s) that pairs with it. Maybe something like: /* * __ww_mutex_lock_check_stamp() will observe our wounded store. */ > > > - if (likely(!(atomic_long_read(&lock->base.owner) & MUTEX_FLAG_WAITERS))) > > > + if (likely(list_empty(&lock->base.wait_list))) > > > return; > > > /* > > > @@ -653,6 +695,17 @@ __ww_mutex_lock_check_stamp(struct mutex *lock, > > > struct mutex_waiter *waiter, > > > struct ww_acquire_ctx *hold_ctx = READ_ONCE(ww->ctx); > > > struct mutex_waiter *cur; > > > + /* > > > + * If we miss a wounded == true here, we will have a pending > > Explain how we can miss that. > > This is actually the pairing location of the wake_up_process() comment / > code discussed above. Here we should have !TASK_RUNNING, and let's say > ctx->wounded is set by another process immediately after we've read it (we > "miss" it). At that point there must be a pending wake-up-process() for us > and we'll pick up the set value of wounded on the next iteration after > returning from schedule(). Right, so that's when the above wakeup isn't the one waking us. > > I can't say I'm a fan. I'm already cursing the ww_mutex stuff every time > > I have to look at it, and you just made it worse spagethi. > Well, I can't speak for the current ww implementation except I didn't think > it was too hard to understand for a first time reader. > > Admittedly the Wound-Wait path makes it worse since it's a preemptive > algorithm and we need to touch other processes a acquire contexts and worry > about ordering. > > So, assuming your review comments are fixed up, is that a solid NAK or do > you have any suggestion that would make you more comfortable with the code? > like splitting out ww-stuff to a separate file? Nah, not a NAK, but we should look at whan can be done to improve code. Maybe add a few more comments that explain why. Part of the problem with ww_mutex is always that I forget exactly how they work and mutex.c doesn't have much useful comments in (most of those are in ww_mutex.h and I always forget to look there). Also; I'm not at all sure about the exact difference between what we have and what you propose. I did read the documentation part (I really should not have to) but it just doesn't jive. I suspect you're using preemption entirely different from what we usually call a preemption. Also, __ww_ctx_stamp_after() is crap; did we want to write: return (signed long)(a->stamp - b->stamp) > 0; or something? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mxsfb: rename driver to mxsfb-drm
[adding Sascha and Bartlomiej] On 13.06.2018 09:17, Marek Vasut wrote: > On 06/13/2018 09:16 AM, Stefan Agner wrote: >> On 13.06.2018 01:08, Marek Vasut wrote: >>> On 06/12/2018 06:01 PM, Fabio Estevam wrote: On Tue, Jun 12, 2018 at 11:35 AM, Stefan Agner wrote: > There are two drivers for the LCDIF/MXSFB peripheral. If the DRM > and fbdev are compiled in, the registration of the second driver > fails: > mxs-dma 3300.dma-apbh: initialized > mxsfb 3073.lcdif: 3073.lcdif supply lcd not found, using dummy > regulator > mxsfb 3073.lcdif: initialized > Error: Driver 'mxsfb' is already registered, aborting... > > Avoid driver name conflict with MXS fbdev driver by renaming the > more recently added DRM driver. > > Signed-off-by: Stefan Agner Yes, this driver name collision is annoying: Reviewed-by: Fabio Estevam >>> Why dont we just kill the old fbdev driver instead ? >> >> The config is outside of my direct control, it's a distro kernel... >> >> Or what do you mean exactly? Send a patch to remove the fbdev driver? > > Yes, remove the fbdev driver. Hm, forcing people to use the DRM driver with fbdev emulation is probably fine for most use cases. Currently you cannot use the fbdev ioctrl to change modes/settings to the framebuffer which is probably the only restriction... It seems to me a rather extreme measure though, given we could fix the situation rather easily. But I am ok to remove the fbdev driver, if that is what we want. -- Stefan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105145] vaExportSurfaceHandle interaction with surface interlaced flag prevents switching on vaapi deinterlacing dynamically
https://bugs.freedesktop.org/show_bug.cgi?id=105145 --- Comment #13 from k.phil...@gmail.com --- (In reply to Christian König from comment #12) > Unfortunately yes it is. That is indeed very unfortunate. It complicates stuff a lot :-( I'd still like to see a solution that allows to support PAFF without copying around already-decoded frames/fields all the time. Do you have any ideas? Could we have the decoder put progressive content into progressive surfaces and interlaced content into interlaced surfaces (under the assumption that we will most likely want to deinterlace)? > That HEVC and VP9 only support progressive layout in the output format is > also only logical because those formats don't support interlaced content. At least HEVC does support interlaced content in theory, but as far as I understood it the (hw) decoder does not need special support for that. How would that work then, would we get top/bottom field as two separate quasi-frames? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 200045] black screen on 'radeon' module probing
https://bugzilla.kernel.org/show_bug.cgi?id=200045 --- Comment #3 from cerg2010cerg2...@mail.ru --- I did some more research and it turns out that 2) happens if I change brightness level right after X starts, even in 4.15 kernel. If I wait a bit, everything works good, including 4.17.1. Software I2C doesn't reproduce same results, so I'm not sure if I should report 2) because it's stated that it can cause issues, but with different kind of errors: [drm] hw_i2c forced on, you may experience display detection problems! -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] locking: Implement an algorithm choice for Wound-Wait mutexes
On 06/13/2018 11:50 AM, Peter Zijlstra wrote: + + lockdep_assert_held(&lock->wait_lock); + + if (owner && hold_ctx && __ww_ctx_stamp_after(hold_ctx, ww_ctx) && + ww_ctx->acquired > 0) { + WRITE_ONCE(hold_ctx->wounded, true); + if (owner != current) { + /* +* wake_up_process() inserts a write memory barrier to It does no such thing. But yes, it does ensure the wakee sees all prior stores IFF the wakeup happened. +* make sure owner sees it is wounded before +* TASK_RUNNING in case it's sleeping on another +* ww_mutex. Note that owner points to a valid +* task_struct as long as we hold the wait_lock. +*/ What exactly are you trying to say here ? I'm thinking this is the pairing barrier to the smp_mb() below, with your list_empty() thing? Might make sense to write a single coherent comment and refer to the other location. So what I'm trying to say here is that wake_up_process() ensures that the owner, if in !TASK_RUNNING, sees the write to hold_ctx->wounded before the transition to TASK_RUNNING. This was how I interpreted "woken up" in the wake up process documentation. + wake_up_process(owner); + } + return true; + } + + return false; +} + /* * Wake up any waiters that may have to back off when the lock is held by the * given context. * * Due to the invariants on the wait list, this can only affect the first - * waiter with a context. + * waiter with a context, unless the Wound-Wait algorithm is used where + * also subsequent waiters with a context main wound the lock holder. * * The current task must not be on the wait list. */ @@ -303,6 +338,7 @@ static void __sched __ww_mutex_wakeup_for_backoff(struct mutex *lock, struct ww_acquire_ctx *ww_ctx) { struct mutex_waiter *cur; + bool is_wait_die = ww_ctx->ww_class->is_wait_die; lockdep_assert_held(&lock->wait_lock); @@ -310,13 +346,14 @@ __ww_mutex_wakeup_for_backoff(struct mutex *lock, struct ww_acquire_ctx *ww_ctx) if (!cur->ww_ctx) continue; - if (cur->ww_ctx->acquired > 0 && + if (is_wait_die && cur->ww_ctx->acquired > 0 && __ww_ctx_stamp_after(cur->ww_ctx, ww_ctx)) { debug_mutex_wake_waiter(lock, cur); wake_up_process(cur->task); } - break; + if (is_wait_die || __ww_mutex_wound(lock, cur->ww_ctx, ww_ctx)) + break; } } @@ -338,12 +375,17 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) * and keep spinning, or it will acquire wait_lock, add itself * to waiter list and sleep. */ - smp_mb(); /* ^^^ */ + smp_mb(); /* See comments above and below. */ /* -* Check if lock is contended, if not there is nobody to wake up +* Check if lock is contended, if not there is nobody to wake up. +* Checking MUTEX_FLAG_WAITERS is not enough here, That seems like a superfluous thing to say. It makes sense in the context of this patch because we change the FLAG check into a list check, but the resulting comment/code looks odd. since we need to +* order against the lock->ctx check in __ww_mutex_wound called from +* __ww_mutex_add_waiter. We can use list_empty without taking the +* wait_lock, given the memory barrier above and the list_empty +* documentation. I don't trust documentation. Please reason about implementation. Will do. */ - if (likely(!(atomic_long_read(&lock->base.owner) & MUTEX_FLAG_WAITERS))) + if (likely(list_empty(&lock->base.wait_list))) return; /* @@ -653,6 +695,17 @@ __ww_mutex_lock_check_stamp(struct mutex *lock, struct mutex_waiter *waiter, struct ww_acquire_ctx *hold_ctx = READ_ONCE(ww->ctx); struct mutex_waiter *cur; + /* +* If we miss a wounded == true here, we will have a pending Explain how we can miss that. This is actually the pairing location of the wake_up_process() comment / code discussed above. Here we should have !TASK_RUNNING, and let's say ctx->wounded is set by another process immediately after we've read it (we "miss" it). At that point there must be a pending wake-up-process() for us and we'll pick up the set value of wounded on the next iteration after returning from schedule(). +* TASK_RUNNING and pick it up on the next schedule fall-through. +*/ + if (!ctx->ww_class->is_wait_die) { + if (READ_ONCE(ctx->wounded)) + goto deadlock; + else +
Re: [PATCH v10 3/3] drm: writeback: Add client capability for exposing writeback connectors
Hi Liviu, On Tue, Jun 12, 2018 at 02:52:33PM +0100, Liviu Dudau wrote: Due to the fact that writeback connectors behave in a special way in DRM (they always report being disconnected) we might confuse some userspace. Add a client capability for writeback connectors that will filter them out for clients that don't understand the capability. Changelog: - only accept the capability if the client has already set the DRM_CLIENT_CAP_ATOMIC one. Cc: Sean Paul Cc: Brian Starkey Signed-off-by: Liviu Dudau LGTM, Reviewed-by: Brian Starkey Thanks, -Brian --- drivers/gpu/drm/drm_ioctl.c | 7 +++ drivers/gpu/drm/drm_mode_config.c | 5 + include/drm/drm_file.h| 7 +++ include/uapi/drm/drm.h| 9 + 4 files changed, 28 insertions(+) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 0d4cfb232576f..fe49fb0356b5a 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -334,6 +334,13 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) return -EINVAL; file_priv->aspect_ratio_allowed = req->value; break; + case DRM_CLIENT_CAP_WRITEBACK_CONNECTORS: + if (!file_priv->atomic) + return -EINVAL; + if (req->value > 1) + return -EINVAL; + file_priv->writeback_connectors = req->value; + break; default: return -EINVAL; } diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index e5c653357024d..21e353bd3948e 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -145,6 +145,11 @@ int drm_mode_getresources(struct drm_device *dev, void *data, count = 0; connector_id = u64_to_user_ptr(card_res->connector_id_ptr); drm_for_each_connector_iter(connector, &conn_iter) { + /* only expose writeback connectors if userspace understands them */ + if (!file_priv->writeback_connectors && + (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)) + continue; + if (drm_lease_held(file_priv, connector->base.id)) { if (count < card_res->count_connectors && put_user(connector->base.id, connector_id + count)) { diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 027ac16da3d15..26485acc51d79 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -192,6 +192,13 @@ struct drm_file { */ unsigned aspect_ratio_allowed:1; + /** +* @writeback_connectors: +* +* True if client understands writeback connectors +*/ + unsigned writeback_connectors:1; + /** * @is_master: * diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 9c660e1688abe..300f336633f28 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -687,6 +687,15 @@ struct drm_get_cap { */ #define DRM_CLIENT_CAP_ASPECT_RATIO4 +/** + * DRM_CLIENT_CAP_WRITEBACK_CONNECTORS + * + * If set to 1, the DRM core will expose special connectors to be used for + * writing back to memory the scene setup in the commit. Depends on client + * also supporting DRM_CLIENT_CAP_ATOMIC + */ +#define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS5 + /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */ struct drm_set_client_cap { __u64 capability; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] locking: Implement an algorithm choice for Wound-Wait mutexes
/me wonders what's up with partial Cc's today.. On Wed, Jun 13, 2018 at 09:47:44AM +0200, Thomas Hellstrom wrote: > The current Wound-Wait mutex algorithm is actually not Wound-Wait but > Wait-Die. Implement also Wound-Wait as a per-ww-class choice. Wound-Wait > is, contrary to Wait-Die a preemptive algorithm and is known to generate > fewer backoffs. Testing reveals that this is true if the > number of simultaneous contending transactions is small. > As the number of simultaneous contending threads increases, Wait-Wound > becomes inferior to Wait-Die in terms of elapsed time. > Possibly due to the larger number of held locks of sleeping transactions. > > Update documentation and callers. > > Timings using git://people.freedesktop.org/~thomash/ww_mutex_test > tag patch-18-06-04 > > Each thread runs 10 batches of lock / unlock 800 ww mutexes randomly > chosen out of 10. Four core Intel x86_64: > > Algorithm#threads Rollbacks time > Wound-Wait 4 ~100 ~17s. > Wait-Die 4 ~15~19s. > Wound-Wait 16 ~36~109s. > Wait-Die 16 ~45~82s. > diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h > index 39fda195bf78..6278077f288b 100644 > --- a/include/linux/ww_mutex.h > +++ b/include/linux/ww_mutex.h > @@ -8,6 +8,8 @@ > * > * Wound/wait implementation: > * Copyright (C) 2013 Canonical Ltd. > + * Choice of algorithm: > + * Copyright (C) 2018 WMWare Inc. > * > * This file contains the main data structure and API definitions. > */ > @@ -23,15 +25,17 @@ struct ww_class { > struct lock_class_key mutex_key; > const char *acquire_name; > const char *mutex_name; > + bool is_wait_die; > }; No _Bool in composites please. > struct ww_acquire_ctx { > struct task_struct *task; > unsigned long stamp; > unsigned acquired; > + bool wounded; Again. > + struct ww_class *ww_class; > #ifdef CONFIG_DEBUG_MUTEXES > unsigned done_acquire; > - struct ww_class *ww_class; > struct ww_mutex *contending_lock; > #endif > #ifdef CONFIG_DEBUG_LOCK_ALLOC > diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c > index 2048359f33d2..b449a012c6f9 100644 > --- a/kernel/locking/mutex.c > +++ b/kernel/locking/mutex.c > @@ -290,12 +290,47 @@ __ww_ctx_stamp_after(struct ww_acquire_ctx *a, struct > ww_acquire_ctx *b) > (a->stamp != b->stamp || a > b); > } > > +/* > + * Wound the lock holder transaction if it's younger than the contending > + * transaction, and there is a possibility of a deadlock. > + * Also if the lock holder transaction isn't the current transaction, Comma followed by a capital? > + * Make sure it's woken up in case it's sleeping on another ww mutex. > + */ > +static bool __ww_mutex_wound(struct mutex *lock, > + struct ww_acquire_ctx *ww_ctx, > + struct ww_acquire_ctx *hold_ctx) > +{ > + struct task_struct *owner = > + __owner_task(atomic_long_read(&lock->owner)); Did you just spell __mutex_owner() wrong? > + > + lockdep_assert_held(&lock->wait_lock); > + > + if (owner && hold_ctx && __ww_ctx_stamp_after(hold_ctx, ww_ctx) && > + ww_ctx->acquired > 0) { > + WRITE_ONCE(hold_ctx->wounded, true); > + if (owner != current) { > + /* > + * wake_up_process() inserts a write memory barrier to It does no such thing. But yes, it does ensure the wakee sees all prior stores IFF the wakeup happened. > + * make sure owner sees it is wounded before > + * TASK_RUNNING in case it's sleeping on another > + * ww_mutex. Note that owner points to a valid > + * task_struct as long as we hold the wait_lock. > + */ What exactly are you trying to say here ? I'm thinking this is the pairing barrier to the smp_mb() below, with your list_empty() thing? Might make sense to write a single coherent comment and refer to the other location. > + wake_up_process(owner); > + } > + return true; > + } > + > + return false; > +} > + > /* > * Wake up any waiters that may have to back off when the lock is held by the > * given context. > * > * Due to the invariants on the wait list, this can only affect the first > - * waiter with a context. > + * waiter with a context, unless the Wound-Wait algorithm is used where > + * also subsequent waiters with a context main wound the lock holder. > * > * The current task must not be on the wait list. > */ > @@ -303,6 +338,7 @@ static void __sched > __ww_mutex_wakeup_for_backoff(struct mutex *lock, struct ww_acquire_ctx > *ww_ctx) > { > struct mutex_waiter *cur; > + bool is_wait_die = ww_ctx->ww_class->is_wait_die; > > lockdep_assert_held(&lock->wait_lock); > > @@
[Bug 200045] black screen on 'radeon' module probing
https://bugzilla.kernel.org/show_bug.cgi?id=200045 --- Comment #2 from Wolfram Sang (w...@the-dreams.de) --- Okay, these are actually two bug reports meanwhile: 1) 3e5f06bed72fe72166a6778f630241a893f67799 causes a regression when bit-banging I2C in software and results in keeping the screen black. 2) There is some regression when using the a HW interface for I2C which causes a reboot when changing the brightness. I'd suggest to work only on 1) here and create a new bug report for 2). I should be able to give you some patches to test in a few minutes. Thanks for the report and the bisecting! -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 19/28] drm/mediatek: add connection from RDMA2 to DPI1
Hi, Stu: On Wed, 2018-06-13 at 16:58 +0800, Stu Hsieh wrote: > Hi, CK: > > On Wed, 2018-06-13 at 16:14 +0800, CK Hu wrote: > > Hi, Stu: > > > > On Wed, 2018-06-13 at 16:01 +0800, Stu Hsieh wrote: > > > Hi, CK: > > > > > > > > > On Wed, 2018-06-13 at 15:13 +0800, CK Hu wrote: > > > > Hi, Stu: > > > > > > > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > > > > This patch add the connection from RDMA2 to DPI1 > > > > > > > > > > Signed-off-by: Stu Hsieh > > > > > --- > > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 8 > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > > index 31a0832ef9ec..2d883815d79c 100644 > > > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > > @@ -93,9 +93,11 @@ > > > > > #define RDMA1_MOUT_DPI0 0x2 > > > > > #define RDMA1_MOUT_DPI1 0x3 > > > > > #define RDMA2_MOUT_DPI0 0x2 > > > > > +#define RDMA2_MOUT_DPI1 0x3 > > > > > > > > Usually, each bit of a mout register represent a output enable. Is this > > > > value 0x3 a correct value? > > > > > > > > Regards, > > > > CK > > > > > > > In HW CONFIG SPEC or MT2712_E2_MMSYS_Change_note show as following: > > > > > > Bit(s)NameDescription > > > 2:0 DISP_RDMA2_SOUT_SEL_IN 0: output to dsi0 > > > 1: outptu to dsi1 > > > 2: output to dpi0 > > > 3: output to dpi1 > > > 4: output to dsi2 > > > 5: output to dsi3 > > > > > > So, 0x3 is correct value. > > > > The data sheet use the term SOUT match its function, so I think driver > > have better change the naming to SOUT. > > > > Regards, > > CK > > > > The definition DISP_REG_CONFIG_DISP_RDMA2_SOUT is use term SOUT in this > patch. > I know, but RDMA2_MOUT_DPI1 should be changed to RDMA2_SOUT_DPI1. Regards, CK > Regard, > Stu > > > > > > > Regard, > > > Stu > > > > > > > > #define DPI0_SEL_IN_RDMA10x1 > > > > > #define DPI0_SEL_IN_RDMA20x3 > > > > > #define DPI1_SEL_IN_RDMA1(0x1 << 8) > > > > > +#define DPI1_SEL_IN_RDMA2(0x3 << 8) > > > > > #define DSI1_SEL_IN_RDMA10x1 > > > > > #define DSI2_SEL_IN_RDMA1(0x1 << 16) > > > > > #define DSI3_SEL_IN_RDMA1(0x1 << 16) > > > > > @@ -199,6 +201,9 @@ static unsigned int mtk_ddp_mout_en(enum > > > > > mtk_ddp_comp_id cur, > > > > > } else if (cur == DDP_COMPONENT_RDMA2 && next == > > > > > DDP_COMPONENT_DPI0) { > > > > > *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT; > > > > > value = RDMA2_MOUT_DPI0; > > > > > + } else if (cur == DDP_COMPONENT_RDMA2 && next == > > > > > DDP_COMPONENT_DPI1) { > > > > > + *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT; > > > > > + value = RDMA2_MOUT_DPI1; > > > > > } else { > > > > > value = 0; > > > > > } > > > > > @@ -233,6 +238,9 @@ static unsigned int mtk_ddp_sel_in(enum > > > > > mtk_ddp_comp_id cur, > > > > > } else if (cur == DDP_COMPONENT_RDMA2 && next == > > > > > DDP_COMPONENT_DPI0) { > > > > > *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > > > value = DPI0_SEL_IN_RDMA2; > > > > > + } else if (cur == DDP_COMPONENT_RDMA2 && next == > > > > > DDP_COMPONENT_DPI1) { > > > > > + *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > > > + value = DPI1_SEL_IN_RDMA2; > > > > > } else if (cur == DDP_COMPONENT_OVL1 && next == > > > > > DDP_COMPONENT_COLOR1) { > > > > > *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN; > > > > > value = COLOR1_SEL_IN_OVL1; > > > > > > > > > > > > > > > > > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 19/28] drm/mediatek: add connection from RDMA2 to DPI1
Hi, CK: On Wed, 2018-06-13 at 16:14 +0800, CK Hu wrote: > Hi, Stu: > > On Wed, 2018-06-13 at 16:01 +0800, Stu Hsieh wrote: > > Hi, CK: > > > > > > On Wed, 2018-06-13 at 15:13 +0800, CK Hu wrote: > > > Hi, Stu: > > > > > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > > > This patch add the connection from RDMA2 to DPI1 > > > > > > > > Signed-off-by: Stu Hsieh > > > > --- > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 8 > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > index 31a0832ef9ec..2d883815d79c 100644 > > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > @@ -93,9 +93,11 @@ > > > > #define RDMA1_MOUT_DPI00x2 > > > > #define RDMA1_MOUT_DPI10x3 > > > > #define RDMA2_MOUT_DPI00x2 > > > > +#define RDMA2_MOUT_DPI10x3 > > > > > > Usually, each bit of a mout register represent a output enable. Is this > > > value 0x3 a correct value? > > > > > > Regards, > > > CK > > > > > In HW CONFIG SPEC or MT2712_E2_MMSYS_Change_note show as following: > > > > Bit(s) NameDescription > > 2:0 DISP_RDMA2_SOUT_SEL_IN 0: output to dsi0 > > 1: outptu to dsi1 > > 2: output to dpi0 > > 3: output to dpi1 > > 4: output to dsi2 > > 5: output to dsi3 > > > > So, 0x3 is correct value. > > The data sheet use the term SOUT match its function, so I think driver > have better change the naming to SOUT. > > Regards, > CK > The definition DISP_REG_CONFIG_DISP_RDMA2_SOUT is use term SOUT in this patch. Regard, Stu > > > > Regard, > > Stu > > > > > > #define DPI0_SEL_IN_RDMA1 0x1 > > > > #define DPI0_SEL_IN_RDMA2 0x3 > > > > #define DPI1_SEL_IN_RDMA1 (0x1 << 8) > > > > +#define DPI1_SEL_IN_RDMA2 (0x3 << 8) > > > > #define DSI1_SEL_IN_RDMA1 0x1 > > > > #define DSI2_SEL_IN_RDMA1 (0x1 << 16) > > > > #define DSI3_SEL_IN_RDMA1 (0x1 << 16) > > > > @@ -199,6 +201,9 @@ static unsigned int mtk_ddp_mout_en(enum > > > > mtk_ddp_comp_id cur, > > > > } else if (cur == DDP_COMPONENT_RDMA2 && next == > > > > DDP_COMPONENT_DPI0) { > > > > *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT; > > > > value = RDMA2_MOUT_DPI0; > > > > + } else if (cur == DDP_COMPONENT_RDMA2 && next == > > > > DDP_COMPONENT_DPI1) { > > > > + *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT; > > > > + value = RDMA2_MOUT_DPI1; > > > > } else { > > > > value = 0; > > > > } > > > > @@ -233,6 +238,9 @@ static unsigned int mtk_ddp_sel_in(enum > > > > mtk_ddp_comp_id cur, > > > > } else if (cur == DDP_COMPONENT_RDMA2 && next == > > > > DDP_COMPONENT_DPI0) { > > > > *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > > value = DPI0_SEL_IN_RDMA2; > > > > + } else if (cur == DDP_COMPONENT_RDMA2 && next == > > > > DDP_COMPONENT_DPI1) { > > > > + *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > > + value = DPI1_SEL_IN_RDMA2; > > > > } else if (cur == DDP_COMPONENT_OVL1 && next == > > > > DDP_COMPONENT_COLOR1) { > > > > *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN; > > > > value = COLOR1_SEL_IN_OVL1; > > > > > > > > > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/28] drm/mediatek: add connection from RDMA1 to DPI1
Hi, CK: On Wed, 2018-06-13 at 16:27 +0800, CK Hu wrote: > Hi, Stu: > > On Wed, 2018-06-13 at 15:56 +0800, Stu Hsieh wrote: > > Hi, CK: > > > > On Wed, 2018-06-13 at 14:13 +0800, CK Hu wrote: > > > Hi, Stu: > > > > > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > > > This patch add the connection from RDMA1 to DPI1 > > > > > > > > Signed-off-by: Stu Hsieh > > > > --- > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 8 > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > index fed1b5704355..4abd5dabeccf 100644 > > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > @@ -85,7 +85,9 @@ > > > > #define RDMA0_MOUT_DSI20x4 > > > > #define RDMA0_MOUT_DSI30x5 > > > > #define RDMA1_MOUT_DPI00x2 > > > > +#define RDMA1_MOUT_DPI10x3 > > > > > > Usually, each bit of a mout register represent a output enable. Is this > > > value 0x3 a correct value? > > > > > > Regards, > > > CK > > > > > In HW CONFIG SPEC show as following > > > > Bit(s) NameDescription > > 2:0 DISP_PATH1_SOUT_SEL_IN 0 : Output to DSI0 > > 1: Ooutput to DSI1 > > 2: Ooutput to DPI > > 3: Ooutput to DPI1 > > 4: Ooutput to DSI2 > > 5: Ooutput to DSI3 > > 6 : reserved > > 7: Ooutput to DISP_UFOE > > > > So, 0x3 is correct value > > It looks like that RDMA1 output is also SOUT, use the naming SOUT would > be better. > > Regards, > CK > OK, I would change the definition name from DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN to DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN Regard, STu > > > > Regard, > > Stu > > > > > > > > #define DPI0_SEL_IN_RDMA1 0x1 > > > > +#define DPI1_SEL_IN_RDMA1 (0x1 << 8) > > > > #define COLOR1_SEL_IN_OVL1 0x1 > > > > > > > > #define OVL_MOUT_EN_RDMA 0x1 > > > > @@ -171,6 +173,9 @@ static unsigned int mtk_ddp_mout_en(enum > > > > mtk_ddp_comp_id cur, > > > > } else if (cur == DDP_COMPONENT_RDMA1 && next == > > > > DDP_COMPONENT_DPI0) { > > > > *addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN; > > > > value = RDMA1_MOUT_DPI0; > > > > + } else if (cur == DDP_COMPONENT_RDMA1 && next == > > > > DDP_COMPONENT_DPI1) { > > > > + *addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN; > > > > + value = RDMA1_MOUT_DPI1; > > > > } else { > > > > value = 0; > > > > } > > > > @@ -190,6 +195,9 @@ static unsigned int mtk_ddp_sel_in(enum > > > > mtk_ddp_comp_id cur, > > > > } else if (cur == DDP_COMPONENT_RDMA1 && next == > > > > DDP_COMPONENT_DPI0) { > > > > *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > > value = DPI0_SEL_IN_RDMA1; > > > > + } else if (cur == DDP_COMPONENT_RDMA1 && next == > > > > DDP_COMPONENT_DPI1) { > > > > + *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > > + value = DPI1_SEL_IN_RDMA1; > > > > } else if (cur == DDP_COMPONENT_OVL1 && next == > > > > DDP_COMPONENT_COLOR1) { > > > > *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN; > > > > value = COLOR1_SEL_IN_OVL1; > > > > > > > > > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/28] drm/mediatek: add connection from RDMA0 to DSI3
Hi, CK: On Wed, 2018-06-13 at 16:05 +0800, CK Hu wrote: > Hi, Stu: > > On Wed, 2018-06-13 at 15:46 +0800, Stu Hsieh wrote: > > Hi, CK: > > > > On Wed, 2018-06-13 at 13:45 +0800, CK Hu wrote: > > > Hi, Stu: > > > > > > Two inline comment. > > > > > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > > > This patch add the connection from RDMA0 to DSI3 > > > > > > > > Signed-off-by: Stu Hsieh > > > > --- > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 4 > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 +- > > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > index c08aed8dae44..fed1b5704355 100644 > > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > > @@ -83,6 +83,7 @@ > > > > #define GAMMA_MOUT_EN_RDMA10x1 > > > > #define RDMA0_MOUT_DPI00x2 > > > > #define RDMA0_MOUT_DSI20x4 > > > > +#define RDMA0_MOUT_DSI30x5 > > > > > > Usually, each bit of a mout register represent a output enable. Is this > > > value 0x5 is a correct value? > > > > In hw CONFIG SPEC show as following: > > Bit(s) NameDescription > > 2:0 DISP_PATH0_SOUT_SEL_IN 0 : Output to DSI0 > > 1: Ooutput to DSI1 > > 2: Ooutput to DPI > > 3: Ooutput to DPI1 > > 4: Ooutput to DSI2 > > 5: Ooutput to DSI3 > > 6 : reserved > > 7: Ooutput to DISP_UFOE > > So, the value 0x5 is correct value. > > > > From the definition, it looks like that RDMA0 could only single output > (output to only one destination at one moment). The register naming > 'DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN' (MOUT means output to multiple > destination simultaneously) would confuse me. If the data sheet use the > confused naming, I think I could just accept it. > > Regards, > CK > OK, I would change the definition name from DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN to DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN > > Regard, > > Stu > > > > > > > > > #define RDMA1_MOUT_DPI00x2 > > > > #define DPI0_SEL_IN_RDMA1 0x1 > > > > #define COLOR1_SEL_IN_OVL1 0x1 > > > > @@ -164,6 +165,9 @@ static unsigned int mtk_ddp_mout_en(enum > > > > mtk_ddp_comp_id cur, > > > > } else if (cur == DDP_COMPONENT_RDMA0 && next == > > > > DDP_COMPONENT_DSI2) { > > > > *addr = DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN; > > > > value = RDMA0_MOUT_DSI2; > > > > + } else if (cur == DDP_COMPONENT_RDMA0 && next == > > > > DDP_COMPONENT_DSI3) { > > > > + *addr = DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN; > > > > + value = RDMA0_MOUT_DSI3; > > > > } else if (cur == DDP_COMPONENT_RDMA1 && next == > > > > DDP_COMPONENT_DPI0) { > > > > *addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN; > > > > value = RDMA1_MOUT_DPI0; > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > > > index fe6fdc021fc7..22f4c72fa785 100644 > > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > > > @@ -228,7 +228,7 @@ static const struct mtk_ddp_comp_match > > > > mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = { > > > > [DDP_COMPONENT_DSI0]= { MTK_DSI,0, NULL }, > > > > [DDP_COMPONENT_DSI1]= { MTK_DSI,1, NULL }, > > > > [DDP_COMPONENT_DSI2]= { MTK_DSI,2, NULL }, > > > > - [DDP_COMPONENT_DSI2]= { MTK_DSI,3, NULL }, > > > > + [DDP_COMPONENT_DSI3]= { MTK_DSI,3, NULL }, > > > > > > I think this is not related to this patch. > > OK > > > > > > > > Regards, > > > CK > > > > > > > [DDP_COMPONENT_GAMMA] = { MTK_DISP_GAMMA, 0, &ddp_gamma }, > > > > [DDP_COMPONENT_OD0] = { MTK_DISP_OD,0, &ddp_od }, > > > > [DDP_COMPONENT_OD1] = { MTK_DISP_OD,1, &ddp_od }, > > > > > > > > > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] locking: Implement an algorithm choice for Wound-Wait mutexes
On 06/13/2018 09:54 AM, Greg Kroah-Hartman wrote: On Wed, Jun 13, 2018 at 09:47:44AM +0200, Thomas Hellstrom wrote: - +The algorithm (Wait-Die vs Wound-Wait) is chosen using the _is_wait_die +argument to DEFINE_WW_CLASS(). As a rough rule of thumb, use Wound-Wait iff you +typically expect the number of simultaneous competing transactions to be small, +and the rollback cost can be substantial. + Three different ways to acquire locks within the same w/w class. Common definitions for methods #1 and #2: -static DEFINE_WW_CLASS(ww_class); +static DEFINE_WW_CLASS(ww_class, false); Minor nit on the api here. Having a "flag" is a royal pain. You have to go and look up exactly what that "true/false" means every time you run across it in code to figure out what it means. Don't do that if at all possible. Make a new api: DEFINE_WW_CLASS_DIE(ww_class); instead that then wraps that boolean internally to switch between the different types. That way the api is "self-documenting" and we all know what is going on without having to dig through a header file. thanks, greg k-h Good point. I'll update in a v2. Thanks, Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/28] drm/mediatek: add connection from RDMA1 to DPI1
Hi, Stu: On Wed, 2018-06-13 at 15:56 +0800, Stu Hsieh wrote: > Hi, CK: > > On Wed, 2018-06-13 at 14:13 +0800, CK Hu wrote: > > Hi, Stu: > > > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > > This patch add the connection from RDMA1 to DPI1 > > > > > > Signed-off-by: Stu Hsieh > > > --- > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 8 > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > index fed1b5704355..4abd5dabeccf 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > @@ -85,7 +85,9 @@ > > > #define RDMA0_MOUT_DSI2 0x4 > > > #define RDMA0_MOUT_DSI3 0x5 > > > #define RDMA1_MOUT_DPI0 0x2 > > > +#define RDMA1_MOUT_DPI1 0x3 > > > > Usually, each bit of a mout register represent a output enable. Is this > > value 0x3 a correct value? > > > > Regards, > > CK > > > In HW CONFIG SPEC show as following > > Bit(s)NameDescription > 2:0 DISP_PATH1_SOUT_SEL_IN 0 : Output to DSI0 > 1: Ooutput to DSI1 > 2: Ooutput to DPI > 3: Ooutput to DPI1 > 4: Ooutput to DSI2 > 5: Ooutput to DSI3 > 6 : reserved > 7: Ooutput to DISP_UFOE > > So, 0x3 is correct value It looks like that RDMA1 output is also SOUT, use the naming SOUT would be better. Regards, CK > > Regard, > Stu > > > > > #define DPI0_SEL_IN_RDMA10x1 > > > +#define DPI1_SEL_IN_RDMA1(0x1 << 8) > > > #define COLOR1_SEL_IN_OVL1 0x1 > > > > > > #define OVL_MOUT_EN_RDMA 0x1 > > > @@ -171,6 +173,9 @@ static unsigned int mtk_ddp_mout_en(enum > > > mtk_ddp_comp_id cur, > > > } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) { > > > *addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN; > > > value = RDMA1_MOUT_DPI0; > > > + } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) { > > > + *addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN; > > > + value = RDMA1_MOUT_DPI1; > > > } else { > > > value = 0; > > > } > > > @@ -190,6 +195,9 @@ static unsigned int mtk_ddp_sel_in(enum > > > mtk_ddp_comp_id cur, > > > } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) { > > > *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > value = DPI0_SEL_IN_RDMA1; > > > + } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) { > > > + *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > + value = DPI1_SEL_IN_RDMA1; > > > } else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) { > > > *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN; > > > value = COLOR1_SEL_IN_OVL1; > > > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: RE: [Intel-gfx] DRM Inquiry
Hi Jani, The end goal was already achieve by the advice you gave the DRM_DP_AUX_CHARDEV.I just like to extend my knowledge into DRM such as a scenario having a kernel version that doesn't have the DRM_DP_AUX_CHARDEV yet. Would it possible to implement specific DRM_DP_AUX_CHARDEV to it. Thanks,John On Wednesday, June 13, 2018, 3:07:14 PM GMT+8, Jani Nikula wrote: On Wed, 13 Jun 2018, John Sledge wrote: > I like to understand how the DRM_DP_AUX_CHARDEV=y kick off. Try 'git grep DRM_DP_AUX_CHARDEV' in your kernel git repo, and see how it affects conditional compilation. This list isn't kernel development 101. You still didn't say what your end goal is. Forget everything about DP AUX and the chardev and so on, just tell us what you're trying to achieve. Maybe you're asking about X, but you really want to know about Y. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Revert "drm/sun4i: Handle DRM_BUS_FLAG_PIXDATA_*EDGE"
This reverts commit 2c17a4368aad2b88b68e4390c819e226cf320f70. The offending commit triggers a run-time fault when accessing the panel element of the sun4i_tcon structure when no such panel is attached. It was apparently assumed in said commit that a panel is always used with the TCON. Although it is often the case, this is not always true. For instance a bridge might be used instead of a panel. This issue was discovered using an A13-OLinuXino, that uses the TCON in RGB mode for a simple DAC-based VGA bridge. Cc: sta...@vger.kernel.org Signed-off-by: Paul Kocialkowski --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 25 - 1 file changed, 25 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index c3d92d537240..8045871335b5 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -17,7 +17,6 @@ #include #include #include -#include #include @@ -350,9 +349,6 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon, static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon, const struct drm_display_mode *mode) { - struct drm_panel *panel = tcon->panel; - struct drm_connector *connector = panel->connector; - struct drm_display_info display_info = connector->display_info; unsigned int bp, hsync, vsync; u8 clk_delay; u32 val = 0; @@ -410,27 +406,6 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon, if (mode->flags & DRM_MODE_FLAG_PVSYNC) val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE; - /* -* On A20 and similar SoCs, the only way to achieve Positive Edge -* (Rising Edge), is setting dclk clock phase to 2/3(240°). -* By default TCON works in Negative Edge(Falling Edge), -* this is why phase is set to 0 in that case. -* Unfortunately there's no way to logically invert dclk through -* IO_POL register. -* The only acceptable way to work, triple checked with scope, -* is using clock phase set to 0° for Negative Edge and set to 240° -* for Positive Edge. -* On A33 and similar SoCs there would be a 90° phase option, -* but it divides also dclk by 2. -* Following code is a way to avoid quirks all around TCON -* and DOTCLOCK drivers. -*/ - if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE) - clk_set_phase(tcon->dclk, 240); - - if (display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE) - clk_set_phase(tcon->dclk, 0); - regmap_update_bits(tcon->regs, SUN4I_TCON0_IO_POL_REG, SUN4I_TCON0_IO_POL_HSYNC_POSITIVE | SUN4I_TCON0_IO_POL_VSYNC_POSITIVE, val); -- 2.17.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 19/28] drm/mediatek: add connection from RDMA2 to DPI1
Hi, Stu: On Wed, 2018-06-13 at 16:01 +0800, Stu Hsieh wrote: > Hi, CK: > > > On Wed, 2018-06-13 at 15:13 +0800, CK Hu wrote: > > Hi, Stu: > > > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > > This patch add the connection from RDMA2 to DPI1 > > > > > > Signed-off-by: Stu Hsieh > > > --- > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 8 > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > index 31a0832ef9ec..2d883815d79c 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > @@ -93,9 +93,11 @@ > > > #define RDMA1_MOUT_DPI0 0x2 > > > #define RDMA1_MOUT_DPI1 0x3 > > > #define RDMA2_MOUT_DPI0 0x2 > > > +#define RDMA2_MOUT_DPI1 0x3 > > > > Usually, each bit of a mout register represent a output enable. Is this > > value 0x3 a correct value? > > > > Regards, > > CK > > > In HW CONFIG SPEC or MT2712_E2_MMSYS_Change_note show as following: > > Bit(s)NameDescription > 2:0 DISP_RDMA2_SOUT_SEL_IN 0: output to dsi0 > 1: outptu to dsi1 > 2: output to dpi0 > 3: output to dpi1 > 4: output to dsi2 > 5: output to dsi3 > > So, 0x3 is correct value. The data sheet use the term SOUT match its function, so I think driver have better change the naming to SOUT. Regards, CK > > Regard, > Stu > > > > #define DPI0_SEL_IN_RDMA10x1 > > > #define DPI0_SEL_IN_RDMA20x3 > > > #define DPI1_SEL_IN_RDMA1(0x1 << 8) > > > +#define DPI1_SEL_IN_RDMA2(0x3 << 8) > > > #define DSI1_SEL_IN_RDMA10x1 > > > #define DSI2_SEL_IN_RDMA1(0x1 << 16) > > > #define DSI3_SEL_IN_RDMA1(0x1 << 16) > > > @@ -199,6 +201,9 @@ static unsigned int mtk_ddp_mout_en(enum > > > mtk_ddp_comp_id cur, > > > } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) { > > > *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT; > > > value = RDMA2_MOUT_DPI0; > > > + } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) { > > > + *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT; > > > + value = RDMA2_MOUT_DPI1; > > > } else { > > > value = 0; > > > } > > > @@ -233,6 +238,9 @@ static unsigned int mtk_ddp_sel_in(enum > > > mtk_ddp_comp_id cur, > > > } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) { > > > *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > value = DPI0_SEL_IN_RDMA2; > > > + } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) { > > > + *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > > + value = DPI1_SEL_IN_RDMA2; > > > } else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) { > > > *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN; > > > value = COLOR1_SEL_IN_OVL1; > > > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 200045] black screen on 'radeon' module probing
https://bugzilla.kernel.org/show_bug.cgi?id=200045 --- Comment #1 from cerg2010cerg2...@mail.ru --- Ok, actually this is 4.17 issue; on 4.16 hw_i2c=1 fixes the issue completely, but on 4.17 changing backlight in Xorg (using keys) causes a reboot, fbcon works fine though. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 200045] black screen on 'radeon' module probing
https://bugzilla.kernel.org/show_bug.cgi?id=200045 cerg2010cerg2...@mail.ru changed: What|Removed |Added Summary|i2c-algo-bit: black screen |black screen on 'radeon' |on 'radeon' module probing |module probing -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 26/28] drm/mediatek: add DPI1/DSI1/DSI2/DSI3 in comp_init
Hi, CK: On Wed, 2018-06-13 at 15:35 +0800, CK Hu wrote: > Hi, Stu: > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > This patch add components DPI1/DSI1/DSI2/DSI3 in comp_init. > > Because the some parameter for these components initialized > > in their driver. > > > > Signed-off-by: Stu Hsieh > > --- > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > index 22f4c72fa785..ff974d82a4a6 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > @@ -278,7 +278,11 @@ int mtk_ddp_comp_init(struct device *dev, struct > > device_node *node, > > > > if (comp_id == DDP_COMPONENT_BLS || > > comp_id == DDP_COMPONENT_DPI0 || > > + comp_id == DDP_COMPONENT_DPI1 || > > Why not move this modification to the patch 'add component DPI1'? > > > comp_id == DDP_COMPONENT_DSI0 || > > + comp_id == DDP_COMPONENT_DSI1 || > > + comp_id == DDP_COMPONENT_DSI2 || > > Why not move this modification to the patch 'add component DSI2'? > > > + comp_id == DDP_COMPONENT_DSI3 || > > Why not move this modification to the patch 'add component DSI3'? > > Regards, > CK > ok Regard, Stu > > comp_id == DDP_COMPONENT_PWM0) { > > comp->regs = NULL; > > comp->clk = NULL; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/28] drm/mediatek: add connection from RDMA0 to DSI3
Hi, Stu: On Wed, 2018-06-13 at 15:46 +0800, Stu Hsieh wrote: > Hi, CK: > > On Wed, 2018-06-13 at 13:45 +0800, CK Hu wrote: > > Hi, Stu: > > > > Two inline comment. > > > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > > This patch add the connection from RDMA0 to DSI3 > > > > > > Signed-off-by: Stu Hsieh > > > --- > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 4 > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 +- > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > index c08aed8dae44..fed1b5704355 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > > @@ -83,6 +83,7 @@ > > > #define GAMMA_MOUT_EN_RDMA1 0x1 > > > #define RDMA0_MOUT_DPI0 0x2 > > > #define RDMA0_MOUT_DSI2 0x4 > > > +#define RDMA0_MOUT_DSI3 0x5 > > > > Usually, each bit of a mout register represent a output enable. Is this > > value 0x5 is a correct value? > > In hw CONFIG SPEC show as following: > Bit(s)NameDescription > 2:0 DISP_PATH0_SOUT_SEL_IN 0 : Output to DSI0 > 1: Ooutput to DSI1 > 2: Ooutput to DPI > 3: Ooutput to DPI1 > 4: Ooutput to DSI2 > 5: Ooutput to DSI3 > 6 : reserved > 7: Ooutput to DISP_UFOE > So, the value 0x5 is correct value. > From the definition, it looks like that RDMA0 could only single output (output to only one destination at one moment). The register naming 'DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN' (MOUT means output to multiple destination simultaneously) would confuse me. If the data sheet use the confused naming, I think I could just accept it. Regards, CK > Regard, > Stu > > > > > > #define RDMA1_MOUT_DPI0 0x2 > > > #define DPI0_SEL_IN_RDMA10x1 > > > #define COLOR1_SEL_IN_OVL1 0x1 > > > @@ -164,6 +165,9 @@ static unsigned int mtk_ddp_mout_en(enum > > > mtk_ddp_comp_id cur, > > > } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI2) { > > > *addr = DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN; > > > value = RDMA0_MOUT_DSI2; > > > + } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI3) { > > > + *addr = DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN; > > > + value = RDMA0_MOUT_DSI3; > > > } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) { > > > *addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN; > > > value = RDMA1_MOUT_DPI0; > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > > index fe6fdc021fc7..22f4c72fa785 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > > @@ -228,7 +228,7 @@ static const struct mtk_ddp_comp_match > > > mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = { > > > [DDP_COMPONENT_DSI0]= { MTK_DSI,0, NULL }, > > > [DDP_COMPONENT_DSI1]= { MTK_DSI,1, NULL }, > > > [DDP_COMPONENT_DSI2]= { MTK_DSI,2, NULL }, > > > - [DDP_COMPONENT_DSI2]= { MTK_DSI,3, NULL }, > > > + [DDP_COMPONENT_DSI3]= { MTK_DSI,3, NULL }, > > > > I think this is not related to this patch. > OK > > > > > Regards, > > CK > > > > > [DDP_COMPONENT_GAMMA] = { MTK_DISP_GAMMA, 0, &ddp_gamma }, > > > [DDP_COMPONENT_OD0] = { MTK_DISP_OD,0, &ddp_od }, > > > [DDP_COMPONENT_OD1] = { MTK_DISP_OD,1, &ddp_od }, > > > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 14/27] drm/sun4i: Don't check for panel or bridge on TV TCONs
On Wed, Jun 13, 2018 at 3:46 PM, Maxime Ripard wrote: > On Tue, Jun 12, 2018 at 10:00:23PM +0200, Jernej Skrabec wrote: >> TV TCONs are always connected to TV or HDMI encoder, so it doesn't make >> sense to check if panel or bridge is connected to them. >> >> Check if TCON has channel 0 and only then check for connected panel or >> bridges. >> >> Signed-off-by: Jernej Skrabec >> --- >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> index b1205a7bc20f..c9ffa5381185 100644 >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> @@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops = { >> static int sun4i_tcon_probe(struct platform_device *pdev) >> { >> struct device_node *node = pdev->dev.of_node; >> + const struct sun4i_tcon_quirks *quirks; >> struct drm_bridge *bridge; >> struct drm_panel *panel; >> int ret; >> >> - ret = drm_of_find_panel_or_bridge(node, 1, 0, &panel, &bridge); >> - if (ret == -EPROBE_DEFER) >> - return ret; >> + quirks = of_device_get_match_data(&pdev->dev); > > We should probably check ofr the pointer value before dereferencing it. I think we've discussed this before. If the driver has data structures for all the supported compatible strings, and it is device tree only, then we should just let it blow up in the user's face, since they are obviously doing something they shouldn't be doing to get the driver to probe without a compatible string match. ChenYu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 19/28] drm/mediatek: add connection from RDMA2 to DPI1
Hi, CK: On Wed, 2018-06-13 at 15:13 +0800, CK Hu wrote: > Hi, Stu: > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > This patch add the connection from RDMA2 to DPI1 > > > > Signed-off-by: Stu Hsieh > > --- > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > index 31a0832ef9ec..2d883815d79c 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > @@ -93,9 +93,11 @@ > > #define RDMA1_MOUT_DPI00x2 > > #define RDMA1_MOUT_DPI10x3 > > #define RDMA2_MOUT_DPI00x2 > > +#define RDMA2_MOUT_DPI10x3 > > Usually, each bit of a mout register represent a output enable. Is this > value 0x3 a correct value? > > Regards, > CK > In HW CONFIG SPEC or MT2712_E2_MMSYS_Change_note show as following: Bit(s) NameDescription 2:0 DISP_RDMA2_SOUT_SEL_IN 0: output to dsi0 1: outptu to dsi1 2: output to dpi0 3: output to dpi1 4: output to dsi2 5: output to dsi3 So, 0x3 is correct value. Regard, Stu > > #define DPI0_SEL_IN_RDMA1 0x1 > > #define DPI0_SEL_IN_RDMA2 0x3 > > #define DPI1_SEL_IN_RDMA1 (0x1 << 8) > > +#define DPI1_SEL_IN_RDMA2 (0x3 << 8) > > #define DSI1_SEL_IN_RDMA1 0x1 > > #define DSI2_SEL_IN_RDMA1 (0x1 << 16) > > #define DSI3_SEL_IN_RDMA1 (0x1 << 16) > > @@ -199,6 +201,9 @@ static unsigned int mtk_ddp_mout_en(enum > > mtk_ddp_comp_id cur, > > } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) { > > *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT; > > value = RDMA2_MOUT_DPI0; > > + } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) { > > + *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT; > > + value = RDMA2_MOUT_DPI1; > > } else { > > value = 0; > > } > > @@ -233,6 +238,9 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id > > cur, > > } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) { > > *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > value = DPI0_SEL_IN_RDMA2; > > + } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) { > > + *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > + value = DPI1_SEL_IN_RDMA2; > > } else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) { > > *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN; > > value = COLOR1_SEL_IN_OVL1; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/28] drm/mediatek: add connection from RDMA1 to DPI1
Hi, CK: On Wed, 2018-06-13 at 14:13 +0800, CK Hu wrote: > Hi, Stu: > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > This patch add the connection from RDMA1 to DPI1 > > > > Signed-off-by: Stu Hsieh > > --- > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > index fed1b5704355..4abd5dabeccf 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > @@ -85,7 +85,9 @@ > > #define RDMA0_MOUT_DSI20x4 > > #define RDMA0_MOUT_DSI30x5 > > #define RDMA1_MOUT_DPI00x2 > > +#define RDMA1_MOUT_DPI10x3 > > Usually, each bit of a mout register represent a output enable. Is this > value 0x3 a correct value? > > Regards, > CK > In HW CONFIG SPEC show as following Bit(s) NameDescription 2:0 DISP_PATH1_SOUT_SEL_IN 0 : Output to DSI0 1: Ooutput to DSI1 2: Ooutput to DPI 3: Ooutput to DPI1 4: Ooutput to DSI2 5: Ooutput to DSI3 6 : reserved 7: Ooutput to DISP_UFOE So, 0x3 is correct value Regard, Stu > > #define DPI0_SEL_IN_RDMA1 0x1 > > +#define DPI1_SEL_IN_RDMA1 (0x1 << 8) > > #define COLOR1_SEL_IN_OVL1 0x1 > > > > #define OVL_MOUT_EN_RDMA 0x1 > > @@ -171,6 +173,9 @@ static unsigned int mtk_ddp_mout_en(enum > > mtk_ddp_comp_id cur, > > } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) { > > *addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN; > > value = RDMA1_MOUT_DPI0; > > + } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) { > > + *addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN; > > + value = RDMA1_MOUT_DPI1; > > } else { > > value = 0; > > } > > @@ -190,6 +195,9 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id > > cur, > > } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) { > > *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > value = DPI0_SEL_IN_RDMA1; > > + } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) { > > + *addr = DISP_REG_CONFIG_DPI_SEL_IN; > > + value = DPI1_SEL_IN_RDMA1; > > } else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) { > > *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN; > > value = COLOR1_SEL_IN_OVL1; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] locking: Implement an algorithm choice for Wound-Wait mutexes
On Wed, Jun 13, 2018 at 09:47:44AM +0200, Thomas Hellstrom wrote: > - > > +The algorithm (Wait-Die vs Wound-Wait) is chosen using the _is_wait_die > +argument to DEFINE_WW_CLASS(). As a rough rule of thumb, use Wound-Wait iff > you > +typically expect the number of simultaneous competing transactions to be > small, > +and the rollback cost can be substantial. > + > Three different ways to acquire locks within the same w/w class. Common > definitions for methods #1 and #2: > > -static DEFINE_WW_CLASS(ww_class); > +static DEFINE_WW_CLASS(ww_class, false); Minor nit on the api here. Having a "flag" is a royal pain. You have to go and look up exactly what that "true/false" means every time you run across it in code to figure out what it means. Don't do that if at all possible. Make a new api: DEFINE_WW_CLASS_DIE(ww_class); instead that then wraps that boolean internally to switch between the different types. That way the api is "self-documenting" and we all know what is going on without having to dig through a header file. thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] locking: Implement an algorithm choice for Wound-Wait mutexes
The current Wound-Wait mutex algorithm is actually not Wound-Wait but Wait-Die. Implement also Wound-Wait as a per-ww-class choice. Wound-Wait is, contrary to Wait-Die a preemptive algorithm and is known to generate fewer backoffs. Testing reveals that this is true if the number of simultaneous contending transactions is small. As the number of simultaneous contending threads increases, Wait-Wound becomes inferior to Wait-Die in terms of elapsed time. Possibly due to the larger number of held locks of sleeping transactions. Update documentation and callers. Timings using git://people.freedesktop.org/~thomash/ww_mutex_test tag patch-18-06-04 Each thread runs 10 batches of lock / unlock 800 ww mutexes randomly chosen out of 10. Four core Intel x86_64: Algorithm#threads Rollbacks time Wound-Wait 4 ~100 ~17s. Wait-Die 4 ~15~19s. Wound-Wait 16 ~36~109s. Wait-Die 16 ~45~82s. Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Jonathan Corbet Cc: Gustavo Padovan Cc: Maarten Lankhorst Cc: Sean Paul Cc: David Airlie Cc: Davidlohr Bueso Cc: "Paul E. McKenney" Cc: Josh Triplett Cc: Thomas Gleixner Cc: Kate Stewart Cc: Philippe Ombredanne Cc: Greg Kroah-Hartman Cc: linux-...@vger.kernel.org Cc: linux-me...@vger.kernel.org Cc: linaro-mm-...@lists.linaro.org Signed-off-by: Thomas Hellstrom --- Documentation/locking/ww-mutex-design.txt | 57 ++ drivers/dma-buf/reservation.c | 2 +- drivers/gpu/drm/drm_modeset_lock.c| 2 +- include/linux/ww_mutex.h | 19 -- kernel/locking/locktorture.c | 2 +- kernel/locking/mutex.c| 98 --- kernel/locking/test-ww_mutex.c| 2 +- lib/locking-selftest.c| 2 +- 8 files changed, 152 insertions(+), 32 deletions(-) diff --git a/Documentation/locking/ww-mutex-design.txt b/Documentation/locking/ww-mutex-design.txt index 34c3a1b50b9a..29c85623b551 100644 --- a/Documentation/locking/ww-mutex-design.txt +++ b/Documentation/locking/ww-mutex-design.txt @@ -1,4 +1,4 @@ -Wait/Wound Deadlock-Proof Mutex Design +Wound/Wait Deadlock-Proof Mutex Design == Please read mutex-design.txt first, as it applies to wait/wound mutexes too. @@ -32,10 +32,23 @@ the oldest task) wins, and the one with the higher reservation id (i.e. the younger task) unlocks all of the buffers that it has already locked, and then tries again. -In the RDBMS literature this deadlock handling approach is called wait/wound: -The older tasks waits until it can acquire the contended lock. The younger tasks -needs to back off and drop all the locks it is currently holding, i.e. the -younger task is wounded. +In the RDBMS literature, a reservation ticket is associated with a transaction. +and the deadlock handling approach is called Wait-Die. The name is based on +the actions of a locking thread when it encounters an already locked mutex. +If the transaction holding the lock is younger, the locking transaction waits. +If the transaction holding the lock is older, the locking transaction backs off +and dies. Hence Wait-Die. +There is also another algorithm called Wound-Wait: +If the transaction holding the lock is younger, the locking transaction +preempts the transaction holding the lock, requiring it to back off. It +Wounds the other transaction. +If the transaction holding the lock is older, it waits for the other +transaction. Hence Wound-Wait. +The two algorithms are both fair in that a transaction will eventually succeed. +However, the Wound-Wait algorithm is typically stated to generate fewer backoffs +compared to Wait-Die, but is, on the other hand, associated with more work than +Wait-Die when recovering from a backoff. Wound-Wait is also a preemptive +algorithm which requires a reliable way to preempt another transaction. Concepts @@ -47,10 +60,12 @@ Acquire context: To ensure eventual forward progress it is important the a task trying to acquire locks doesn't grab a new reservation id, but keeps the one it acquired when starting the lock acquisition. This ticket is stored in the acquire context. Furthermore the acquire context keeps track of debugging state -to catch w/w mutex interface abuse. +to catch w/w mutex interface abuse. An acquire context is representing a +transaction. W/w class: In contrast to normal mutexes the lock class needs to be explicit for -w/w mutexes, since it is required to initialize the acquire context. +w/w mutexes, since it is required to initialize the acquire context. The lock +class also specifies what algorithm to use, Wound-Wait or Wait-Die. Furthermore there are three different class of w/w lock acquire functions: @@ -90,10 +105,15 @@ provided. Usage - +The algorithm (Wait-Die vs Wound-Wait) is chosen using the _is_wait_die +argument to DEFINE_WW_CLAS
[PATCH 2/2] drm: Change deadlock-avoidance algorithm for the modeset locks.
For modeset locks we don't expect a high number of contending transactions so change algorithm from Wait-Die to Wound-Wait. Signed-off-by: Thomas Hellstrom --- drivers/gpu/drm/drm_modeset_lock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index f22a7ef41de1..294997765a2c 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -70,7 +70,7 @@ * lists and lookup data structures. */ -static DEFINE_WW_CLASS(crtc_ww_class, true); +static DEFINE_WW_CLASS(crtc_ww_class, false); /** * drm_modeset_lock_all - take all modeset locks -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] locking, drm: Fix ww mutex naming / algorithm inconsistency
This is a small fallout from a work to allow batching WW mutex locks and unlocks. Our Wound-Wait mutexes actually don't use the Wound-Wait algorithm but the Wait-Die algorithm. One could perhaps rename those mutexes tree-wide to "Wait-Die mutexes" or "Deadlock Avoidance mutexes". Another approach suggested here is to implement also the "Wound-Wait" algorithm as a per-WW-class choice, as it has advantages in some cases. See for example http://www.mathcs.emory.edu/~cheung/Courses/554/Syllabus/8-recv+serial/deadlock-compare.html Now Wound-Wait is a preemptive algorithm, and the preemption is implemented using a lazy scheme: If a wounded transaction is about to go to sleep on a contended WW mutex, we return -EDEADLK. That is sufficient for deadlock prevention. Since with WW mutexes we also require the aborted transaction to sleep waiting to lock the WW mutex it was aborted on, this choice also provides a suitable WW mutex to sleep on. If we were to return -EDEADLK on the first WW mutex lock after the transaction was wounded whether the WW mutex was contended or not, the transaction might frequently be restarted without a wait, which is far from optimal. Note also that with the lazy preemption scheme, contrary to Wait-Die there will be no rollbacks on lock contention of locks held by a transaction that has completed its locking sequence. The modeset locks are then changed from Wait-Die to Wound-Wait since the typical locking pattern of those locks very well matches the criterion for a substantial reduction in the number of rollbacks. For reservation objects, the benefit is more unclear at this point and they remain using Wait-Die. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 28/28] drm/mediatek: Add support for mediatek SOC MT2712
Hi, Stu: On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > This patch add support for the Mediatek MT2712 DISP subsystem. > There are two OVL engine and three disp output in MT2712. > > Signed-off-by: Stu Hsieh Reviewed-by: CK Hu > --- > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 39 > ++ > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 38 + > 2 files changed, 77 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > index 28dd8531a7de..c3fa5591bfc8 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > @@ -65,6 +65,24 @@ > #define MT8173_MUTEX_MOD_DISP_PWM1 24 > #define MT8173_MUTEX_MOD_DISP_OD 25 > > +#define MT2712_MUTEX_MOD_DISP_PWM2 10 > +#define MT2712_MUTEX_MOD_DISP_OVL0 11 > +#define MT2712_MUTEX_MOD_DISP_OVL1 12 > +#define MT2712_MUTEX_MOD_DISP_RDMA0 13 > +#define MT2712_MUTEX_MOD_DISP_RDMA1 14 > +#define MT2712_MUTEX_MOD_DISP_RDMA2 15 > +#define MT2712_MUTEX_MOD_DISP_WDMA0 16 > +#define MT2712_MUTEX_MOD_DISP_WDMA1 17 > +#define MT2712_MUTEX_MOD_DISP_COLOR0 18 > +#define MT2712_MUTEX_MOD_DISP_COLOR1 19 > +#define MT2712_MUTEX_MOD_DISP_AAL0 20 > +#define MT2712_MUTEX_MOD_DISP_UFOE 22 > +#define MT2712_MUTEX_MOD_DISP_PWM0 23 > +#define MT2712_MUTEX_MOD_DISP_PWM1 24 > +#define MT2712_MUTEX_MOD_DISP_OD025 > +#define MT2712_MUTEX_MOD2_DISP_AAL1 33 > +#define MT2712_MUTEX_MOD2_DISP_OD1 34 > + > #define MT2701_MUTEX_MOD_DISP_OVL3 > #define MT2701_MUTEX_MOD_DISP_WDMA 6 > #define MT2701_MUTEX_MOD_DISP_COLOR 7 > @@ -138,6 +156,26 @@ static const unsigned int > mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = { > [DDP_COMPONENT_WDMA0] = MT2701_MUTEX_MOD_DISP_WDMA, > }; > > +static const unsigned int mt2712_mutex_mod[DDP_COMPONENT_ID_MAX] = { > + [DDP_COMPONENT_AAL0] = MT2712_MUTEX_MOD_DISP_AAL0, > + [DDP_COMPONENT_AAL1] = MT2712_MUTEX_MOD2_DISP_AAL1, > + [DDP_COMPONENT_COLOR0] = MT2712_MUTEX_MOD_DISP_COLOR0, > + [DDP_COMPONENT_COLOR1] = MT2712_MUTEX_MOD_DISP_COLOR1, > + [DDP_COMPONENT_OD0] = MT2712_MUTEX_MOD_DISP_OD0, > + [DDP_COMPONENT_OD1] = MT2712_MUTEX_MOD2_DISP_OD1, > + [DDP_COMPONENT_OVL0] = MT2712_MUTEX_MOD_DISP_OVL0, > + [DDP_COMPONENT_OVL1] = MT2712_MUTEX_MOD_DISP_OVL1, > + [DDP_COMPONENT_PWM0] = MT2712_MUTEX_MOD_DISP_PWM0, > + [DDP_COMPONENT_PWM1] = MT2712_MUTEX_MOD_DISP_PWM1, > + [DDP_COMPONENT_PWM2] = MT2712_MUTEX_MOD_DISP_PWM2, > + [DDP_COMPONENT_RDMA0] = MT2712_MUTEX_MOD_DISP_RDMA0, > + [DDP_COMPONENT_RDMA1] = MT2712_MUTEX_MOD_DISP_RDMA1, > + [DDP_COMPONENT_RDMA2] = MT2712_MUTEX_MOD_DISP_RDMA2, > + [DDP_COMPONENT_UFOE] = MT2712_MUTEX_MOD_DISP_UFOE, > + [DDP_COMPONENT_WDMA0] = MT2712_MUTEX_MOD_DISP_WDMA0, > + [DDP_COMPONENT_WDMA1] = MT2712_MUTEX_MOD_DISP_WDMA1, > +}; > + > static const unsigned int mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = { > [DDP_COMPONENT_AAL0] = MT8173_MUTEX_MOD_DISP_AAL, > [DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0, > @@ -533,6 +571,7 @@ static int mtk_ddp_remove(struct platform_device *pdev) > > static const struct of_device_id ddp_driver_dt_match[] = { > { .compatible = "mediatek,mt2701-disp-mutex", .data = mt2701_mutex_mod}, > + { .compatible = "mediatek,mt2712-disp-mutex", .data = mt2712_mutex_mod}, > { .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod}, > {}, > }; > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index 3d279a299383..3a866e1d6af4 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -146,6 +146,32 @@ static const enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = > { > DDP_COMPONENT_DPI0, > }; > > +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = { > + DDP_COMPONENT_OVL0, > + DDP_COMPONENT_COLOR0, > + DDP_COMPONENT_AAL0, > + DDP_COMPONENT_OD0, > + DDP_COMPONENT_RDMA0, > + DDP_COMPONENT_DPI0, > + DDP_COMPONENT_PWM0, > +}; > + > +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = { > + DDP_COMPONENT_OVL1, > + DDP_COMPONENT_COLOR1, > + DDP_COMPONENT_AAL1, > + DDP_COMPONENT_OD1, > + DDP_COMPONENT_RDMA1, > + DDP_COMPONENT_DPI1, > + DDP_COMPONENT_PWM1, > +}; > + > +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_third[] = { > + DDP_COMPONENT_RDMA2, > + DDP_COMPONENT_DSI3, > + DDP_COMPONENT_PWM2, > +}; > + > static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = { > DDP_COMPONENT_OVL0, > DDP_COMPONENT_COLOR0, > @@ -173,6 +199,15 @@ static const struct mtk_mmsys_driver_data > mt2701_mmsys_driver_data = { > .shadow_register =
Re: [PATCH v2 14/27] drm/sun4i: Don't check for panel or bridge on TV TCONs
On Tue, Jun 12, 2018 at 10:00:23PM +0200, Jernej Skrabec wrote: > TV TCONs are always connected to TV or HDMI encoder, so it doesn't make > sense to check if panel or bridge is connected to them. > > Check if TCON has channel 0 and only then check for connected panel or > bridges. > > Signed-off-by: Jernej Skrabec > --- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index b1205a7bc20f..c9ffa5381185 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops = { > static int sun4i_tcon_probe(struct platform_device *pdev) > { > struct device_node *node = pdev->dev.of_node; > + const struct sun4i_tcon_quirks *quirks; > struct drm_bridge *bridge; > struct drm_panel *panel; > int ret; > > - ret = drm_of_find_panel_or_bridge(node, 1, 0, &panel, &bridge); > - if (ret == -EPROBE_DEFER) > - return ret; > + quirks = of_device_get_match_data(&pdev->dev); We should probably check ofr the pointer value before dereferencing it. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/28] drm/mediatek: add connection from RDMA0 to DSI3
Hi, CK: On Wed, 2018-06-13 at 13:45 +0800, CK Hu wrote: > Hi, Stu: > > Two inline comment. > > On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > > This patch add the connection from RDMA0 to DSI3 > > > > Signed-off-by: Stu Hsieh > > --- > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 4 > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 +- > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > index c08aed8dae44..fed1b5704355 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > > @@ -83,6 +83,7 @@ > > #define GAMMA_MOUT_EN_RDMA10x1 > > #define RDMA0_MOUT_DPI00x2 > > #define RDMA0_MOUT_DSI20x4 > > +#define RDMA0_MOUT_DSI30x5 > > Usually, each bit of a mout register represent a output enable. Is this > value 0x5 is a correct value? In hw CONFIG SPEC show as following: Bit(s) NameDescription 2:0 DISP_PATH0_SOUT_SEL_IN 0 : Output to DSI0 1: Ooutput to DSI1 2: Ooutput to DPI 3: Ooutput to DPI1 4: Ooutput to DSI2 5: Ooutput to DSI3 6 : reserved 7: Ooutput to DISP_UFOE So, the value 0x5 is correct value. Regard, Stu > > > #define RDMA1_MOUT_DPI00x2 > > #define DPI0_SEL_IN_RDMA1 0x1 > > #define COLOR1_SEL_IN_OVL1 0x1 > > @@ -164,6 +165,9 @@ static unsigned int mtk_ddp_mout_en(enum > > mtk_ddp_comp_id cur, > > } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI2) { > > *addr = DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN; > > value = RDMA0_MOUT_DSI2; > > + } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI3) { > > + *addr = DISP_REG_CONFIG_DISP_RDMA0_MOUT_EN; > > + value = RDMA0_MOUT_DSI3; > > } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) { > > *addr = DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN; > > value = RDMA1_MOUT_DPI0; > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > index fe6fdc021fc7..22f4c72fa785 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > @@ -228,7 +228,7 @@ static const struct mtk_ddp_comp_match > > mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = { > > [DDP_COMPONENT_DSI0]= { MTK_DSI,0, NULL }, > > [DDP_COMPONENT_DSI1]= { MTK_DSI,1, NULL }, > > [DDP_COMPONENT_DSI2]= { MTK_DSI,2, NULL }, > > - [DDP_COMPONENT_DSI2]= { MTK_DSI,3, NULL }, > > + [DDP_COMPONENT_DSI3]= { MTK_DSI,3, NULL }, > > I think this is not related to this patch. OK > > Regards, > CK > > > [DDP_COMPONENT_GAMMA] = { MTK_DISP_GAMMA, 0, &ddp_gamma }, > > [DDP_COMPONENT_OD0] = { MTK_DISP_OD,0, &ddp_od }, > > [DDP_COMPONENT_OD1] = { MTK_DISP_OD,1, &ddp_od }, > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 27/28] drm/mediatek: add third ddp path
On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > This patch create third crtc by third ddp path > > Signed-off-by: Stu Hsieh Reviewed-by: CK Hu > --- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +++ > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 + > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 5 - > 3 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index 658b8dd45b83..2d6aa150a9ff 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -539,6 +539,9 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, > int ret; > int i; > > + if (!path) > + return 0; > + > for (i = 0; i < path_len; i++) { > enum mtk_ddp_comp_id comp_id = path[i]; > struct device_node *node; > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index 08d5d0b47bfe..3d279a299383 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -232,6 +232,11 @@ static int mtk_drm_kms_init(struct drm_device *drm) > if (ret < 0) > goto err_component_unbind; > > + ret = mtk_drm_crtc_create(drm, private->data->third_path, > + private->data->third_len); > + if (ret < 0) > + goto err_component_unbind; > + > /* Use OVL device for all DMA memory allocations */ > np = private->comp_node[private->data->main_path[0]] ?: >private->comp_node[private->data->ext_path[0]]; > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h > b/drivers/gpu/drm/mediatek/mtk_drm_drv.h > index c3378c452c0a..ecc00ca3221d 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h > @@ -17,7 +17,7 @@ > #include > #include "mtk_drm_ddp_comp.h" > > -#define MAX_CRTC 2 > +#define MAX_CRTC 3 > #define MAX_CONNECTOR2 > > struct device; > @@ -33,6 +33,9 @@ struct mtk_mmsys_driver_data { > unsigned int main_len; > const enum mtk_ddp_comp_id *ext_path; > unsigned int ext_len; > + const enum mtk_ddp_comp_id *third_path; > + unsigned int third_len; > + > bool shadow_register; > }; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 24/27] drm: of: Export drm_crtc_port_mask()
On Tue, Jun 12, 2018 at 10:00:33PM +0200, Jernej Skrabec wrote: > Function is useful when drm_of_find_possible_crtcs() can't be used and > custom parsing is needed. This can happen for example when there is a > node with multiple muxes between crtc and encoder. > > Signed-off-by: Jernej Skrabec > --- > drivers/gpu/drm/drm_of.c | 4 ++-- > include/drm/drm_of.h | 8 > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c > index 1fe122461298..2e9cea3287b2 100644 > --- a/drivers/gpu/drm/drm_of.c > +++ b/drivers/gpu/drm/drm_of.c > @@ -22,8 +22,8 @@ static void drm_release_of(struct device *dev, void *data) > * Given a port OF node, return the possible mask of the corresponding > * CRTC within a device's list of CRTCs. Returns zero if not found. > */ > -static uint32_t drm_crtc_port_mask(struct drm_device *dev, > -struct device_node *port) > +uint32_t drm_crtc_port_mask(struct drm_device *dev, > + struct device_node *port) It should probably be exported too? Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 26/28] drm/mediatek: add DPI1/DSI1/DSI2/DSI3 in comp_init
Hi, Stu: On Mon, 2018-06-11 at 11:26 +0800, Stu Hsieh wrote: > This patch add components DPI1/DSI1/DSI2/DSI3 in comp_init. > Because the some parameter for these components initialized > in their driver. > > Signed-off-by: Stu Hsieh > --- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > index 22f4c72fa785..ff974d82a4a6 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > @@ -278,7 +278,11 @@ int mtk_ddp_comp_init(struct device *dev, struct > device_node *node, > > if (comp_id == DDP_COMPONENT_BLS || > comp_id == DDP_COMPONENT_DPI0 || > + comp_id == DDP_COMPONENT_DPI1 || Why not move this modification to the patch 'add component DPI1'? > comp_id == DDP_COMPONENT_DSI0 || > + comp_id == DDP_COMPONENT_DSI1 || > + comp_id == DDP_COMPONENT_DSI2 || Why not move this modification to the patch 'add component DSI2'? > + comp_id == DDP_COMPONENT_DSI3 || Why not move this modification to the patch 'add component DSI3'? Regards, CK > comp_id == DDP_COMPONENT_PWM0) { > comp->regs = NULL; > comp->clk = NULL; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/27] dt-bindings: display: sunxi-drm: Add TCON TOP description
Hi, Thanks for working on this! On Tue, Jun 12, 2018 at 10:00:13PM +0200, Jernej Skrabec wrote: > TCON TOP main purpose is to configure whole display pipeline. It > determines relationships between mixers and TCONs, selects source TCON > for HDMI, muxes LCD and TV encoder GPIO output, selects TV encoder > clock source and contains additional TV TCON and DSI gates. > > Signed-off-by: Jernej Skrabec > --- > .../bindings/display/sunxi/sun4i-drm.txt | 45 +++ > include/dt-bindings/clock/sun8i-tcon-top.h| 11 + > 2 files changed, 56 insertions(+) > create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h > > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > index 3346c1e2a7a0..ef64c589a4b3 100644 > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > @@ -187,6 +187,51 @@ And on the A23, A31, A31s and A33, you need one more > clock line: > - 'lvds-alt': An alternative clock source, separate from the TCON channel > 0 > clock, that can be used to drive the LVDS clock > > +TCON TOP > + > + > +TCON TOPs main purpose is to configure whole display pipeline. It determines > +relationships between mixers and TCONs, selects source TCON for HDMI, muxes > +LCD and TV encoder GPIO output, selects TV encoder clock source and contains > +additional TV TCON and DSI gates. > + > +It allows display pipeline to be configured in very different ways: > + > + / LCD0/LVDS0 > + / TCON-LCD0 > + | \ MIPI DSI > + mixer0 | > +\/ TCON-LCD1 - LCD1/LVDS1 > + TCON-TOP > +/\ TCON-TV0 - TVE0/RGB > + mixer1 | \ > + | TCON-TOP - HDMI > + | / > + \ TCON-TV1 - TVE1/RGB > + > +Note that both TCON TOP references same physical unit. > + > +Required properties: > + - compatible: value must be one of: > +* allwinner,sun8i-r40-tcon-top > + - reg: base address and size of the memory-mapped region. > + - clocks: phandle to the clocks feeding the TCON TOP > +* bus: TCON TOP interface clock > + - clock-names: clock name mentioned above > + - resets: phandle to the reset line driving the DRC s/DRC/TCON TOP/ ? > +* rst: TCON TOP reset line Remaining consistent with the clock name would be great > + - reset-names: reset name mentioned above > + - #clock-cells : must contain 1 An example would be nice here > +- ports: A ports node with endpoint definitions as defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. The first > port > +should be the input for mixer0 mux. The second should be the output for > that > +mux. Third port should be input for mixer1 mux. Fourth port should be > output > +for mixer1 mux. Fifth port should be input for HDMI mux. Sixth port > should > +be output for it. All output endpoints should have reg property with the > id > +of the target TCON. All ports should have only one enpoint connected to ^ endpoint I guess it would me more readable if you were to make it a bullet list, but this works for me otherwise. Thanks! Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 13/27] drm/sun4i: Don't check for LVDS and RGB when TCON has only ch1
LVDS and RGB interfaces are always connected to TCONs which have channel 0. It doesn't make sense to try to init them on TV TCONs. Add a check if TCON has channel 0 before trying to init LVDS or RGB interface. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 32 -- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 5dae623968f7..b1205a7bc20f 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1117,23 +1117,25 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, goto err_free_dotclock; } - /* -* If we have an LVDS panel connected to the TCON, we should -* just probe the LVDS connector. Otherwise, just probe RGB as -* we used to. -*/ - remote = of_graph_get_remote_node(dev->of_node, 1, 0); - if (of_device_is_compatible(remote, "panel-lvds")) - if (can_lvds) - ret = sun4i_lvds_init(drm, tcon); + if (tcon->quirks->has_channel_0) { + /* +* If we have an LVDS panel connected to the TCON, we should +* just probe the LVDS connector. Otherwise, just probe RGB as +* we used to. +*/ + remote = of_graph_get_remote_node(dev->of_node, 1, 0); + if (of_device_is_compatible(remote, "panel-lvds")) + if (can_lvds) + ret = sun4i_lvds_init(drm, tcon); + else + ret = -EINVAL; else - ret = -EINVAL; - else - ret = sun4i_rgb_init(drm, tcon); - of_node_put(remote); + ret = sun4i_rgb_init(drm, tcon); + of_node_put(remote); - if (ret < 0) - goto err_free_dotclock; + if (ret < 0) + goto err_free_dotclock; + } if (tcon->quirks->needs_de_be_mux) { /* -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/9] xen/balloon: Share common memory reservation routines
On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko diff --git a/include/xen/mem-reservation.h b/include/xen/mem-reservation.h new file mode 100644 index ..e0939387278d --- /dev/null +++ b/include/xen/mem-reservation.h @@ -0,0 +1,64 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * Xen memory reservation utilities. + * + * Copyright (c) 2003, B Dragovic + * Copyright (c) 2003-2004, M Williamson, K Fraser + * Copyright (c) 2005 Dan M. Smith, IBM Corporation + * Copyright (c) 2010 Daniel Kiper + * Copyright (c) 2018 Oleksandr Andrushchenko, EPAM Systems Inc. + */ + +#ifndef _XENMEM_RESERVATION_H +#define _XENMEM_RESERVATION_H + +#include +#include + +#include +#include + +#include +#include I should have noticed this in the previous post but I suspect most of these includes belong in the C file. For example, there is no reason for hypercall.h here. -boris + +static inline void xenmem_reservation_scrub_page(struct page *page) +{ +#ifdef CONFIG_XEN_SCRUB_PAGES + clear_highpage(page); +#endif +} + +#ifdef CONFIG_XEN_HAVE_PVMMU +void __xenmem_reservation_va_mapping_update(unsigned long count, + struct page **pages, + xen_pfn_t *frames); + +void __xenmem_reservation_va_mapping_reset(unsigned long count, + struct page **pages); +#endif + +static inline void xenmem_reservation_va_mapping_update(unsigned long count, + struct page **pages, + xen_pfn_t *frames) +{ +#ifdef CONFIG_XEN_HAVE_PVMMU + if (!xen_feature(XENFEAT_auto_translated_physmap)) + __xenmem_reservation_va_mapping_update(count, pages, frames); +#endif +} + +static inline void xenmem_reservation_va_mapping_reset(unsigned long count, + struct page **pages) +{ +#ifdef CONFIG_XEN_HAVE_PVMMU + if (!xen_feature(XENFEAT_auto_translated_physmap)) + __xenmem_reservation_va_mapping_reset(count, pages); +#endif +} + +int xenmem_reservation_increase(int count, xen_pfn_t *frames); + +int xenmem_reservation_decrease(int count, xen_pfn_t *frames); + +#endif ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 11/27] drm/sun4i: tcon: Add support for tcon-top gate
TV TCONs connected to TCON TOP have to enable additional gate in order to work. Add support for such TCONs. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++ drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 2 files changed, 15 insertions(+) diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..0afb5a94a414 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev, dev_err(dev, "Couldn't get the TCON bus clock\n"); return PTR_ERR(tcon->clk); } + + if (tcon->quirks->has_tcon_top_gate) { + tcon->top_clk = devm_clk_get(dev, "tcon-top"); + if (IS_ERR(tcon->top_clk)) { + dev_err(dev, "Couldn't get the TCON TOP bus clock\n"); + return PTR_ERR(tcon->top_clk); + } + clk_prepare_enable(tcon->top_clk); + } + clk_prepare_enable(tcon->clk); if (tcon->quirks->has_channel_0) { @@ -712,6 +722,7 @@ static int sun4i_tcon_init_clocks(struct device *dev, static void sun4i_tcon_free_clocks(struct sun4i_tcon *tcon) { clk_disable_unprepare(tcon->clk); + clk_disable_unprepare(tcon->top_clk); } static int sun4i_tcon_init_irq(struct device *dev, diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index f6a071cd5a6f..652d5c37d7b4 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -224,6 +224,7 @@ struct sun4i_tcon_quirks { boolneeds_de_be_mux; /* sun6i needs mux to select backend */ boolneeds_edp_reset; /* a80 edp reset needed for tcon0 access */ boolsupports_lvds; /* Does the TCON support an LVDS output? */ + boolhas_tcon_top_gate; /* TCON TOP holds additional gate to enable */ /* callback to handle tcon muxing options */ int (*set_mux)(struct sun4i_tcon *, const struct drm_encoder *); @@ -249,6 +250,9 @@ struct sun4i_tcon { u8 dclk_max_div; u8 dclk_min_div; + /* TCON TOP clock */ + struct clk *top_clk; + /* Reset control */ struct reset_control*lcd_rst; struct reset_control*lvds_rst; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 25/27] drm/sun4i: DW HDMI: Expand algorithm for possible crtcs
drm_of_find_possible_crtcs() doesn't work when DW HDMI encoder is connected to TCON (crtc) through mux in TCON TOP. In that case TCON TOP HDMI mux input port has to be manually traversed and checked if it matches any known crtc. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 46 ++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 9f40a44b456b..d443886e055b 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c @@ -12,6 +12,7 @@ #include #include "sun8i_dw_hdmi.h" +#include "sun8i_tcon_top.h" static void sun8i_dw_hdmi_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, @@ -41,6 +42,48 @@ sun8i_dw_hdmi_mode_valid(struct drm_connector *connector, return MODE_OK; } +static bool sun8i_dw_hdmi_node_is_tcon_top(struct device_node *node) +{ + return !!of_match_node(sun8i_tcon_top_of_table, node); +} + +static u32 sun8i_dw_hdmi_find_possible_crtcs(struct drm_device *drm, +struct device_node *node) +{ + struct device_node *port, *ep, *remote, *remote_port; + u32 crtcs = 0; + + port = of_graph_get_port_by_id(node, 0); + if (!port) + return 0; + + ep = of_get_next_available_child(port, NULL); + if (!ep) + return 0; + + remote = of_graph_get_remote_port_parent(ep); + if (!remote) + return 0; + + if (sun8i_dw_hdmi_node_is_tcon_top(remote)) { + port = of_graph_get_port_by_id(remote, 4); + if (!port) + return 0; + + for_each_child_of_node(port, ep) { + remote_port = of_graph_get_remote_port(ep); + if (remote_port) { + crtcs |= drm_crtc_port_mask(drm, remote_port); + of_node_put(remote_port); + } + } + } else { + crtcs = drm_of_find_possible_crtcs(drm, node); + } + + return crtcs; +} + static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, void *data) { @@ -63,7 +106,8 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, hdmi->dev = &pdev->dev; encoder = &hdmi->encoder; - encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node); + encoder->possible_crtcs = + sun8i_dw_hdmi_find_possible_crtcs(drm, dev->of_node); /* * If we failed to find the CRTC(s) which this encoder is * supposed to be connected to, it's because the CRTC has -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 20/27] drm/sun4i: Don't change clock bits in DW HDMI PHY driver
DW HDMI PHY driver and PHY clock driver share same registers. Make sure that DW HDMI PHY setup code doesn't change any clock related bits and set them to 0 during initialization. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 +- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 12 +++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 79154f0f674a..3ba71aff92fc 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h @@ -98,7 +98,7 @@ #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_ENBIT(29) #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_ENBIT(28) #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27) -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26) +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26) #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25) #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x)((x) << 22) #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20) diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index 966688f04741..cd07ceb71601 100644 --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi *hdmi, regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG, SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0); - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init); + /* +* NOTE: We have to be careful not to overwrite PHY parent +* clock selection bit and clock divider. +*/ + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, + (u32)~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK, + pll_cfg1_init); regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK, pll_cfg2_init); @@ -352,6 +358,10 @@ static void sun8i_hdmi_phy_init_h3(struct sun8i_hdmi_phy *phy) SUN8I_HDMI_PHY_ANA_CFG3_SCLEN | SUN8I_HDMI_PHY_ANA_CFG3_SDAEN); + /* reset PLL clock configuration */ + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, 0); + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, 0); + /* set HW control of CEC pins */ regmap_write(phy->regs, SUN8I_HDMI_PHY_CEC_REG, 0); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 17/27] drm/sun4i: Add support for R40 mixers
Both mixers have similar capabilities as others SoCs with DE2. First mixer has 1 VI and 3 UI planes and supports HW scaling on all planes. Second mixer has 1 VI and 1 UI planes and also supports HW scaling on all planes. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 24 1 file changed, 24 insertions(+) diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 126899d6f0d3..ee8febb25903 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -500,6 +500,22 @@ static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = { .vi_num = 1, }; +static const struct sun8i_mixer_cfg sun8i_r40_mixer0_cfg = { + .ccsc = 0, + .mod_rate = 29700, + .scaler_mask= 0xf, + .ui_num = 3, + .vi_num = 1, +}; + +static const struct sun8i_mixer_cfg sun8i_r40_mixer1_cfg = { + .ccsc = 1, + .mod_rate = 29700, + .scaler_mask= 0x3, + .ui_num = 1, + .vi_num = 1, +}; + static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg = { .vi_num = 2, .ui_num = 1, @@ -521,6 +537,14 @@ static const struct of_device_id sun8i_mixer_of_table[] = { .compatible = "allwinner,sun8i-h3-de2-mixer-0", .data = &sun8i_h3_mixer0_cfg, }, + { + .compatible = "allwinner,sun8i-r40-de2-mixer-0", + .data = &sun8i_r40_mixer0_cfg, + }, + { + .compatible = "allwinner,sun8i-r40-de2-mixer-1", + .data = &sun8i_r40_mixer1_cfg, + }, { .compatible = "allwinner,sun8i-v3s-de2-mixer", .data = &sun8i_v3s_mixer_cfg, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 24/27] drm: of: Export drm_crtc_port_mask()
Function is useful when drm_of_find_possible_crtcs() can't be used and custom parsing is needed. This can happen for example when there is a node with multiple muxes between crtc and encoder. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/drm_of.c | 4 ++-- include/drm/drm_of.h | 8 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index 1fe122461298..2e9cea3287b2 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -22,8 +22,8 @@ static void drm_release_of(struct device *dev, void *data) * Given a port OF node, return the possible mask of the corresponding * CRTC within a device's list of CRTCs. Returns zero if not found. */ -static uint32_t drm_crtc_port_mask(struct drm_device *dev, - struct device_node *port) +uint32_t drm_crtc_port_mask(struct drm_device *dev, + struct device_node *port) { unsigned int index = 0; struct drm_crtc *tmp; diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h index b93c239afb60..a61fd77e46ba 100644 --- a/include/drm/drm_of.h +++ b/include/drm/drm_of.h @@ -17,6 +17,8 @@ struct drm_bridge; struct device_node; #ifdef CONFIG_OF +uint32_t drm_crtc_port_mask(struct drm_device *dev, + struct device_node *port); uint32_t drm_of_find_possible_crtcs(struct drm_device *dev, struct device_node *port); void drm_of_component_match_add(struct device *master, @@ -34,6 +36,12 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, struct drm_panel **panel, struct drm_bridge **bridge); #else +static inline uint32_t drm_crtc_port_mask(struct drm_device *dev, + struct device_node *port) +{ + return 0; +} + static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev, struct device_node *port) { -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 4/9] xen/grant-table: Allow allocating buffers suitable for DMA
On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Extend grant table module API to allow allocating buffers that can be used for DMA operations and mapping foreign grant references on top of those. The resulting buffer is similar to the one allocated by the balloon driver in terms that proper memory reservation is made ({increase|decrease}_reservation and VA mappings updated if needed). This is useful for sharing foreign buffers with HW drivers which cannot work with scattered buffers provided by the balloon driver, but require DMAable memory instead. Signed-off-by: Oleksandr Andrushchenko Reviewed-by: Boris Ostrovsky with a small nit below --- drivers/xen/Kconfig | 13 ++ drivers/xen/grant-table.c | 97 +++ include/xen/grant_table.h | 18 3 files changed, 128 insertions(+) diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index e5d0c28372ea..39536ddfbce4 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -161,6 +161,19 @@ config XEN_GRANT_DEV_ALLOC to other domains. This can be used to implement frontend drivers or as part of an inter-domain shared memory channel. +config XEN_GRANT_DMA_ALLOC + bool "Allow allocating DMA capable buffers with grant reference module" + depends on XEN && HAS_DMA + help + Extends grant table module API to allow allocating DMA capable + buffers and mapping foreign grant references on top of it. + The resulting buffer is similar to one allocated by the balloon + driver in terms that proper memory reservation is made + ({increase|decrease}_reservation and VA mappings updated if needed). I think you should drop the word "terms" and say "is made *by*" and "VA mappings *are* updated" And similar change in the commit message. -boris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 23/27] drm/sun4i: Add support for A64 HDMI PHY
PHY is the same as in H3, except it can switch between two clock parents. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index e1b7196d4587..457f0a121684 100644 --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c @@ -396,6 +396,14 @@ static struct regmap_config sun8i_hdmi_phy_regmap_config = { .name = "phy" }; +static const struct sun8i_hdmi_phy_variant sun50i_a64_hdmi_phy = { + .has_phy_clk = true, + .has_second_pll = true, + .phy_init = &sun8i_hdmi_phy_init_h3, + .phy_disable = &sun8i_hdmi_phy_disable_h3, + .phy_config = &sun8i_hdmi_phy_config_h3, +}; + static const struct sun8i_hdmi_phy_variant sun8i_a83t_hdmi_phy = { .phy_init = &sun8i_hdmi_phy_init_a83t, .phy_disable = &sun8i_hdmi_phy_disable_a83t, @@ -410,6 +418,10 @@ static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = { }; static const struct of_device_id sun8i_hdmi_phy_of_table[] = { + { + .compatible = "allwinner,sun50i-a64-hdmi-phy", + .data = &sun50i_a64_hdmi_phy, + }, { .compatible = "allwinner,sun8i-a83t-hdmi-phy", .data = &sun8i_a83t_hdmi_phy, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 09/27] drm/sun4i: Don't skip TCONs if they don't have channel 0
TV TCONs (channel 1 only) are always connected to TV or HDMI encoder. Because of that, all output endpoints on such TCON node will point to a encoder which is part of component framework. Correct current graph traversing algorithm in such way that it doesn't skip output enpoints with id 0 on TV TCONs. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun4i_drv.c | 52 +-- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index e6c62c079146..6ddf4eaccb40 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -198,6 +198,22 @@ static bool sun4i_drv_node_is_tcon(struct device_node *node) return !!of_match_node(sun4i_tcon_of_table, node); } +static bool sun4i_drv_node_is_tcon_with_ch0(struct device_node *node) +{ + const struct of_device_id *match; + + match = of_match_node(sun4i_tcon_of_table, node); + if (match) { + struct sun4i_tcon_quirks *quirks; + + quirks = (struct sun4i_tcon_quirks *)match->data; + + return quirks->has_channel_0; + } + + return false; +} + static bool sun4i_drv_node_is_tcon_top(struct device_node *node) { return !!of_match_node(sun8i_tcon_top_of_table, node); @@ -256,14 +272,7 @@ static void sun4i_drv_traverse_endpoints(struct endpoint_list *list, continue; } - /* -* If the node is our TCON, the first port is used for -* panel or bridges, and will not be part of the -* component framework. -*/ if (sun4i_drv_node_is_tcon(node)) { - struct of_endpoint endpoint; - /* * TCON TOP is always probed before TCON. However, TCON * points back to TCON TOP when it is source for HDMI. @@ -276,16 +285,25 @@ static void sun4i_drv_traverse_endpoints(struct endpoint_list *list, continue; } - if (of_graph_parse_endpoint(ep, &endpoint)) { - DRM_DEBUG_DRIVER("Couldn't parse endpoint\n"); - of_node_put(remote); - continue; - } - - if (!endpoint.id) { - DRM_DEBUG_DRIVER("Endpoint is our panel... skipping\n"); - of_node_put(remote); - continue; + /* +* If the node is our TCON with channel 0, the first +* port is used for panel or bridges, and will not be +* part of the component framework. +*/ + if (sun4i_drv_node_is_tcon_with_ch0(node)) { + struct of_endpoint endpoint; + + if (of_graph_parse_endpoint(ep, &endpoint)) { + DRM_DEBUG_DRIVER("Couldn't parse endpoint\n"); + of_node_put(remote); + continue; + } + + if (!endpoint.id) { + DRM_DEBUG_DRIVER("Endpoint is our panel... skipping\n"); + of_node_put(remote); + continue; + } } } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 6/9] xen/gntdev: Make private routines/structures accessible
On 06/13/2018 04:38 AM, Boris Ostrovsky wrote: On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko This is in preparation for adding support of DMA buffer functionality: make map/unmap related code and structures, used privately by gntdev, ready for dma-buf extension, which will re-use these. Rename corresponding structures as those become non-private to gntdev now. Signed-off-by: Oleksandr Andrushchenko --- drivers/xen/gntdev-common.h | 86 +++ drivers/xen/gntdev.c | 132 2 files changed, 128 insertions(+), 90 deletions(-) create mode 100644 drivers/xen/gntdev-common.h diff --git a/drivers/xen/gntdev-common.h b/drivers/xen/gntdev-common.h new file mode 100644 index ..7a9845a6bee9 --- /dev/null +++ b/drivers/xen/gntdev-common.h @@ -0,0 +1,86 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * Common functionality of grant device. + * + * Copyright (c) 2006-2007, D G Murray. + * (c) 2009 Gerd Hoffmann + * (c) 2018 Oleksandr Andrushchenko, EPAM Systems Inc. + */ + +#ifndef _GNTDEV_COMMON_H +#define _GNTDEV_COMMON_H + +#include +#include +#include +#include + +struct gntdev_priv { + /* maps with visible offsets in the file descriptor */ + struct list_head maps; + /* maps that are not visible; will be freed on munmap. + * Only populated if populate_freeable_maps == 1 */ Since you are touching this code please fix comment style. I saw that while running checkpatch, but was not sure if I have to touch those as they seemed to be not related to the change itself. But I'll make sure all the comments are consistent. + struct list_head freeable_maps; + /* lock protects maps and freeable_maps */ + struct mutex lock; + struct mm_struct *mm; + struct mmu_notifier mn; + +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + /* Device for which DMA memory is allocated. */ + struct device *dma_dev; +#endif +}; With that fixed, Reviewed-by: Boris Ostrovsky Thank you, Oleksandr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 18/27] dt-bindings: display: sun4i-drm: Add description of A64 HDMI PHY
A64 HDMI PHY is similar to H3 HDMI PHY except it has two possible PLL clock parents. It is compatible to other HDMI PHYs, like that found in R40. Signed-off-by: Jernej Skrabec --- Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index d84df6d808c2..6fb45c7a9ac8 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt @@ -101,6 +101,7 @@ DWC HDMI PHY Required properties: - compatible: value must be one of: +* allwinner,sun50i-a64-hdmi-phy * allwinner,sun8i-a83t-hdmi-phy * allwinner,sun8i-h3-hdmi-phy - reg: base address and size of memory-mapped region @@ -111,8 +112,9 @@ Required properties: - resets: phandle to the reset controller driving the PHY - reset-names: must be "phy" -H3 HDMI PHY requires additional clock: +H3 and A64 HDMI PHY require additional clocks: - pll-0: parent of phy clock + - pll-1: second possible phy clock parent (A64 only) TV Encoder -- -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 14/27] drm/sun4i: Don't check for panel or bridge on TV TCONs
TV TCONs are always connected to TV or HDMI encoder, so it doesn't make sense to check if panel or bridge is connected to them. Check if TCON has channel 0 and only then check for connected panel or bridges. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index b1205a7bc20f..c9ffa5381185 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops = { static int sun4i_tcon_probe(struct platform_device *pdev) { struct device_node *node = pdev->dev.of_node; + const struct sun4i_tcon_quirks *quirks; struct drm_bridge *bridge; struct drm_panel *panel; int ret; - ret = drm_of_find_panel_or_bridge(node, 1, 0, &panel, &bridge); - if (ret == -EPROBE_DEFER) - return ret; + quirks = of_device_get_match_data(&pdev->dev); + + /* panels and bridges are present only on TCONs with channel 0 */ + if (quirks->has_channel_0) { + ret = drm_of_find_panel_or_bridge(node, 1, 0, &panel, &bridge); + if (ret == -EPROBE_DEFER) + return ret; + } return component_add(&pdev->dev, &sun4i_tcon_ops); } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 5/9] xen/gntdev: Allow mappings for DMA buffers
On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: static void gntdev_print_maps(struct gntdev_priv *priv, @@ -121,8 +146,27 @@ static void gntdev_free_map(struct grant_map *map) if (map == NULL) return; +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + if (map->dma_vaddr) { + struct gnttab_dma_alloc_args args; + + args.dev = map->dma_dev; + args.coherent = map->dma_flags & GNTDEV_DMA_FLAG_COHERENT; args.coherent = !!(map->dma_flags & GNTDEV_DMA_FLAG_COHERENT); + args.nr_pages = map->count; + args.pages = map->pages; + args.frames = map->frames; + args.vaddr = map->dma_vaddr; + args.dev_bus_addr = map->dma_bus_addr; + + gnttab_dma_free_pages(&args); + } else +#endif if (map->pages) gnttab_free_pages(map->count, map->pages); + +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + kfree(map->frames); +#endif kfree(map->pages); kfree(map->grants); kfree(map->map_ops); @@ -132,7 +176,8 @@ static void gntdev_free_map(struct grant_map *map) kfree(map); } -static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count) +static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count, + int dma_flags) { struct grant_map *add; int i; @@ -155,6 +200,37 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count) NULL == add->pages) goto err; +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + add->dma_flags = dma_flags; + + /* +* Check if this mapping is requested to be backed +* by a DMA buffer. +*/ + if (dma_flags & (GNTDEV_DMA_FLAG_WC | GNTDEV_DMA_FLAG_COHERENT)) { + struct gnttab_dma_alloc_args args; + + add->frames = kcalloc(count, sizeof(add->frames[0]), + GFP_KERNEL); + if (!add->frames) + goto err; + + /* Remember the device, so we can free DMA memory. */ + add->dma_dev = priv->dma_dev; + + args.dev = priv->dma_dev; + args.coherent = dma_flags & GNTDEV_DMA_FLAG_COHERENT; And again here. + args.nr_pages = count; + args.pages = add->pages; + args.frames = add->frames; + + if (gnttab_dma_alloc_pages(&args)) + goto err; + + add->dma_vaddr = args.vaddr; + add->dma_bus_addr = args.dev_bus_addr; + } else +#endif if (gnttab_alloc_pages(count, add->pages)) goto err; @@ -325,6 +401,14 @@ static int map_grant_pages(struct grant_map *map) map->unmap_ops[i].handle = map->map_ops[i].handle; if (use_ptemod) map->kunmap_ops[i].handle = map->kmap_ops[i].handle; +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + else if (map->dma_vaddr) { + unsigned long mfn; This should be called bfn now. + + mfn = pfn_to_bfn(page_to_pfn(map->pages[i])); + map->unmap_ops[i].dev_bus_addr = __pfn_to_phys(mfn); + } +#endif } return err; } @@ -548,6 +632,17 @@ static int gntdev_open(struct inode *inode, struct file *flip) } flip->private_data = priv; +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + priv->dma_dev = gntdev_miscdev.this_device; + + /* +* The device is not spawn from a device tree, so arch_setup_dma_ops +* is not called, thus leaving the device with dummy DMA ops. +* Fix this call of_dma_configure() with a NULL node to set "Fix this by calling ..." I think. +* default DMA ops. +*/ + of_dma_configure(priv->dma_dev, NULL); +#endif pr_debug("priv %p\n", priv); return 0; With those fixed, Reviewed-by: Boris Ostrovsky ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 19/27] drm/sun4i: Enable DW HDMI PHY clock
Current DW HDMI PHY code never prepares and enables PHY clock after it is created. It's just used as it is. This may work in some cases, but it's clearly wrong. Fix it by adding proper calls to enable/disable PHY clock. Fixes: 4f86e81748fe ("drm/sun4i: Add support for H3 HDMI PHY variant") Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index 5a52fc489a9d..966688f04741 100644 --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c @@ -477,13 +477,15 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, struct device_node *node) dev_err(dev, "Couldn't create the PHY clock\n"); goto err_put_clk_pll0; } + + clk_prepare_enable(phy->clk_phy); } phy->rst_phy = of_reset_control_get_shared(node, "phy"); if (IS_ERR(phy->rst_phy)) { dev_err(dev, "Could not get phy reset control\n"); ret = PTR_ERR(phy->rst_phy); - goto err_put_clk_pll0; + goto err_disable_clk_phy; } ret = reset_control_deassert(phy->rst_phy); @@ -514,6 +516,8 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, struct device_node *node) reset_control_assert(phy->rst_phy); err_put_rst_phy: reset_control_put(phy->rst_phy); +err_disable_clk_phy: + clk_disable_unprepare(phy->clk_phy); err_put_clk_pll0: if (phy->variant->has_phy_clk) clk_put(phy->clk_pll0); @@ -531,6 +535,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi *hdmi) clk_disable_unprepare(phy->clk_mod); clk_disable_unprepare(phy->clk_bus); + clk_disable_unprepare(phy->clk_phy); reset_control_assert(phy->rst_phy); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mxsfb: rename driver to mxsfb-drm
On 06/12/2018 06:01 PM, Fabio Estevam wrote: > On Tue, Jun 12, 2018 at 11:35 AM, Stefan Agner wrote: > >> There are two drivers for the LCDIF/MXSFB peripheral. If the DRM >> and fbdev are compiled in, the registration of the second driver >> fails: >> mxs-dma 3300.dma-apbh: initialized >> mxsfb 3073.lcdif: 3073.lcdif supply lcd not found, using dummy >> regulator >> mxsfb 3073.lcdif: initialized >> Error: Driver 'mxsfb' is already registered, aborting... >> >> Avoid driver name conflict with MXS fbdev driver by renaming the >> more recently added DRM driver. >> >> Signed-off-by: Stefan Agner > > Yes, this driver name collision is annoying: > > Reviewed-by: Fabio Estevam > Why dont we just kill the old fbdev driver instead ? -- Best regards, Marek Vasut ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 6/9] xen/gntdev: Make private routines/structures accessible
On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko This is in preparation for adding support of DMA buffer functionality: make map/unmap related code and structures, used privately by gntdev, ready for dma-buf extension, which will re-use these. Rename corresponding structures as those become non-private to gntdev now. Signed-off-by: Oleksandr Andrushchenko --- drivers/xen/gntdev-common.h | 86 +++ drivers/xen/gntdev.c| 132 2 files changed, 128 insertions(+), 90 deletions(-) create mode 100644 drivers/xen/gntdev-common.h diff --git a/drivers/xen/gntdev-common.h b/drivers/xen/gntdev-common.h new file mode 100644 index ..7a9845a6bee9 --- /dev/null +++ b/drivers/xen/gntdev-common.h @@ -0,0 +1,86 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * Common functionality of grant device. + * + * Copyright (c) 2006-2007, D G Murray. + * (c) 2009 Gerd Hoffmann + * (c) 2018 Oleksandr Andrushchenko, EPAM Systems Inc. + */ + +#ifndef _GNTDEV_COMMON_H +#define _GNTDEV_COMMON_H + +#include +#include +#include +#include + +struct gntdev_priv { + /* maps with visible offsets in the file descriptor */ + struct list_head maps; + /* maps that are not visible; will be freed on munmap. +* Only populated if populate_freeable_maps == 1 */ Since you are touching this code please fix comment style. + struct list_head freeable_maps; + /* lock protects maps and freeable_maps */ + struct mutex lock; + struct mm_struct *mm; + struct mmu_notifier mn; + +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + /* Device for which DMA memory is allocated. */ + struct device *dma_dev; +#endif +}; With that fixed, Reviewed-by: Boris Ostrovsky ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 5/9] xen/gntdev: Allow mappings for DMA buffers
On 06/13/2018 04:26 AM, Boris Ostrovsky wrote: On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: static void gntdev_print_maps(struct gntdev_priv *priv, @@ -121,8 +146,27 @@ static void gntdev_free_map(struct grant_map *map) if (map == NULL) return; +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + if (map->dma_vaddr) { + struct gnttab_dma_alloc_args args; + + args.dev = map->dma_dev; + args.coherent = map->dma_flags & GNTDEV_DMA_FLAG_COHERENT; args.coherent = !!(map->dma_flags & GNTDEV_DMA_FLAG_COHERENT); Will fix + args.nr_pages = map->count; + args.pages = map->pages; + args.frames = map->frames; + args.vaddr = map->dma_vaddr; + args.dev_bus_addr = map->dma_bus_addr; + + gnttab_dma_free_pages(&args); + } else +#endif if (map->pages) gnttab_free_pages(map->count, map->pages); + +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + kfree(map->frames); +#endif kfree(map->pages); kfree(map->grants); kfree(map->map_ops); @@ -132,7 +176,8 @@ static void gntdev_free_map(struct grant_map *map) kfree(map); } -static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count) +static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count, + int dma_flags) { struct grant_map *add; int i; @@ -155,6 +200,37 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count) NULL == add->pages) goto err; +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + add->dma_flags = dma_flags; + + /* + * Check if this mapping is requested to be backed + * by a DMA buffer. + */ + if (dma_flags & (GNTDEV_DMA_FLAG_WC | GNTDEV_DMA_FLAG_COHERENT)) { + struct gnttab_dma_alloc_args args; + + add->frames = kcalloc(count, sizeof(add->frames[0]), + GFP_KERNEL); + if (!add->frames) + goto err; + + /* Remember the device, so we can free DMA memory. */ + add->dma_dev = priv->dma_dev; + + args.dev = priv->dma_dev; + args.coherent = dma_flags & GNTDEV_DMA_FLAG_COHERENT; And again here. Will fix + args.nr_pages = count; + args.pages = add->pages; + args.frames = add->frames; + + if (gnttab_dma_alloc_pages(&args)) + goto err; + + add->dma_vaddr = args.vaddr; + add->dma_bus_addr = args.dev_bus_addr; + } else +#endif if (gnttab_alloc_pages(count, add->pages)) goto err; @@ -325,6 +401,14 @@ static int map_grant_pages(struct grant_map *map) map->unmap_ops[i].handle = map->map_ops[i].handle; if (use_ptemod) map->kunmap_ops[i].handle = map->kmap_ops[i].handle; +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + else if (map->dma_vaddr) { + unsigned long mfn; This should be called bfn now. Of course + + mfn = pfn_to_bfn(page_to_pfn(map->pages[i])); + map->unmap_ops[i].dev_bus_addr = __pfn_to_phys(mfn); + } +#endif } return err; } @@ -548,6 +632,17 @@ static int gntdev_open(struct inode *inode, struct file *flip) } flip->private_data = priv; +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC + priv->dma_dev = gntdev_miscdev.this_device; + + /* + * The device is not spawn from a device tree, so arch_setup_dma_ops + * is not called, thus leaving the device with dummy DMA ops. + * Fix this call of_dma_configure() with a NULL node to set "Fix this by calling ..." I think. Will fix + * default DMA ops. + */ + of_dma_configure(priv->dma_dev, NULL); +#endif pr_debug("priv %p\n", priv); return 0; With those fixed, Reviewed-by: Boris Ostrovsky Thank you, Oleksandr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mxsfb: rename driver to mxsfb-drm
On 06/13/2018 09:16 AM, Stefan Agner wrote: > On 13.06.2018 01:08, Marek Vasut wrote: >> On 06/12/2018 06:01 PM, Fabio Estevam wrote: >>> On Tue, Jun 12, 2018 at 11:35 AM, Stefan Agner wrote: >>> There are two drivers for the LCDIF/MXSFB peripheral. If the DRM and fbdev are compiled in, the registration of the second driver fails: mxs-dma 3300.dma-apbh: initialized mxsfb 3073.lcdif: 3073.lcdif supply lcd not found, using dummy regulator mxsfb 3073.lcdif: initialized Error: Driver 'mxsfb' is already registered, aborting... Avoid driver name conflict with MXS fbdev driver by renaming the more recently added DRM driver. Signed-off-by: Stefan Agner >>> >>> Yes, this driver name collision is annoying: >>> >>> Reviewed-by: Fabio Estevam >>> >> Why dont we just kill the old fbdev driver instead ? > > The config is outside of my direct control, it's a distro kernel... > > Or what do you mean exactly? Send a patch to remove the fbdev driver? Yes, remove the fbdev driver. -- Best regards, Marek Vasut ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/9] xen/balloon: Share common memory reservation routines
On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: One more thing: please add a comment here saying that frames array is array of PFNs (in Xen granularity), which is what XENMEM_populate_physmap requires. And remove (or update to name the actual call you are making) the corresponding comment in increase_reservation(). + +int xenmem_reservation_increase(int count, xen_pfn_t *frames) +{ + struct xen_memory_reservation reservation = { + .address_bits = 0, + .extent_order = EXTENT_ORDER, + .domid= DOMID_SELF + }; + + set_xen_guest_handle(reservation.extent_start, frames); + reservation.nr_extents = count; + return HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation); +} +EXPORT_SYMBOL_GPL(xenmem_reservation_increase); And similarly, here we are requesting GFNs, and update decrease_reservation(). -boris + +int xenmem_reservation_decrease(int count, xen_pfn_t *frames) +{ + struct xen_memory_reservation reservation = { + .address_bits = 0, + .extent_order = EXTENT_ORDER, + .domid= DOMID_SELF + }; + + set_xen_guest_handle(reservation.extent_start, frames); + reservation.nr_extents = count; + return HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 9/9] xen/gntdev: Implement dma-buf import functionality
On 06/12/2018 09:42 AM, Oleksandr Andrushchenko wrote: int gntdev_dmabuf_imp_release(struct gntdev_dmabuf_priv *priv, u32 fd) { - return -EINVAL; + struct gntdev_dmabuf *gntdev_dmabuf; + struct dma_buf_attachment *attach; + struct dma_buf *dma_buf; + + gntdev_dmabuf = dmabuf_imp_find_unlink(priv, fd); + if (IS_ERR(gntdev_dmabuf)) + return PTR_ERR(gntdev_dmabuf); + + pr_debug("Releasing DMA buffer with fd %d\n", fd); + + attach = gntdev_dmabuf->u.imp.attach; + + if (gntdev_dmabuf->u.imp.sgt) + dma_buf_unmap_attachment(attach, gntdev_dmabuf->u.imp.sgt, +DMA_BIDIRECTIONAL); + dma_buf = attach->dmabuf; + dma_buf_detach(attach->dmabuf, attach); + dma_buf_put(dma_buf); + + dmabuf_imp_end_foreign_access(gntdev_dmabuf->u.imp.refs, + gntdev_dmabuf->nr_pages); Should you first end foreign access, before doing anything? -boris + dmabuf_imp_free_storage(gntdev_dmabuf); + return 0; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 26/27] ARM: dts: sun8i: r40: Add HDMI pipeline
Add all entries needed for HDMI to function properly. Since R40 has highly configurable pipeline, both mixers and both TCON TVs are added. Board specific DT should then connect them together trough TCON TOP muxers to best fit the purpose of the board. Signed-off-by: Jernej Skrabec --- arch/arm/boot/dts/sun8i-r40.dtsi | 257 +++ 1 file changed, 257 insertions(+) diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi index 173dcc1652d2..17171c82457e 100644 --- a/arch/arm/boot/dts/sun8i-r40.dtsi +++ b/arch/arm/boot/dts/sun8i-r40.dtsi @@ -42,8 +42,11 @@ */ #include +#include #include +#include #include +#include / { #address-cells = <1>; @@ -99,12 +102,76 @@ }; }; + de: display-engine { + compatible = "allwinner,sun8i-r40-display-engine", +"allwinner,sun8i-h3-display-engine"; + allwinner,pipelines = <&mixer0>, <&mixer1>; + status = "disabled"; + }; + soc { compatible = "simple-bus"; #address-cells = <1>; #size-cells = <1>; ranges; + display_clocks: clock@100 { + compatible = "allwinner,sun8i-r40-de2-clk", +"allwinner,sun8i-h3-de2-clk"; + reg = <0x0100 0x10>; + clocks = <&ccu CLK_DE>, +<&ccu CLK_BUS_DE>; + clock-names = "mod", + "bus"; + resets = <&ccu RST_BUS_DE>; + #clock-cells = <1>; + #reset-cells = <1>; + }; + + mixer0: mixer@110 { + compatible = "allwinner,sun8i-r40-de2-mixer-0"; + reg = <0x0110 0x10>; + clocks = <&display_clocks CLK_BUS_MIXER0>, +<&display_clocks CLK_MIXER0>; + clock-names = "bus", + "mod"; + resets = <&display_clocks RST_MIXER0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + mixer0_out: port@1 { + reg = <1>; + mixer0_out_tcon_top: endpoint { + remote-endpoint = <&tcon_top_mixer0_in_mixer0>; + }; + }; + }; + }; + + mixer1: mixer@120 { + compatible = "allwinner,sun8i-r40-de2-mixer-1"; + reg = <0x0120 0x10>; + clocks = <&display_clocks CLK_BUS_MIXER1>, +<&display_clocks CLK_MIXER1>; + clock-names = "bus", + "mod"; + resets = <&display_clocks RST_WB>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + mixer1_out: port@1 { + reg = <1>; + mixer1_out_tcon_top: endpoint { + remote-endpoint = <&tcon_top_mixer1_in_mixer1>; + }; + }; + }; + }; + nmi_intc: interrupt-controller@1c00030 { compatible = "allwinner,sun7i-a20-sc-nmi"; interrupt-controller; @@ -451,6 +518,151 @@ #size-cells = <0>; }; + tcon_top: tcon-top@1c7 { + compatible = "allwinner,sun8i-r40-tcon-top"; + reg = <0x01c7 0x1000>; + clocks = <&ccu CLK_BUS_TCON_TOP>; + clock-names = "bus"; + resets = <&ccu RST_BUS_TCON_TOP>; + reset-names = "rst"; + #clock-cells = <1>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + tcon_top_mixer0_in: port@0 { + reg = <0>; + + tcon_top_mixer0_in_mixer0: endpoint { + remote-endpoint = <&mixer0_out_tcon_top>; + }; + }; + + tcon_top_mixer0_out: port@1
Re: [PATCH v3 8/9] xen/gntdev: Implement dma-buf export functionality
On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko 1. Create a dma-buf from grant references provided by the foreign domain. By default dma-buf is backed by system memory pages, but by providing GNTDEV_DMA_FLAG_XXX flags it can also be created as a DMA write-combine/coherent buffer, e.g. allocated with corresponding dma_alloc_xxx API. Export the resulting buffer as a new dma-buf. 2. Implement waiting for the dma-buf to be released: block until the dma-buf with the file descriptor provided is released. If within the time-out provided the buffer is not released then -ETIMEDOUT error is returned. If the buffer with the file descriptor does not exist or has already been released, then -ENOENT is returned. For valid file descriptors this must not be treated as error. 3. Make gntdev's common code and structures available to dma-buf. Signed-off-by: Oleksandr Andrushchenko --- drivers/xen/gntdev-common.h | 4 + drivers/xen/gntdev-dmabuf.c | 470 +++- drivers/xen/gntdev.c| 10 + 3 files changed, 482 insertions(+), 2 deletions(-) diff --git a/drivers/xen/gntdev-common.h b/drivers/xen/gntdev-common.h index a3408fd39b07..72f80dbce861 100644 --- a/drivers/xen/gntdev-common.h +++ b/drivers/xen/gntdev-common.h @@ -89,4 +89,8 @@ bool gntdev_account_mapped_pages(int count); int gntdev_map_grant_pages(struct gntdev_grant_map *map); +#ifdef CONFIG_XEN_GNTDEV_DMABUF +void gntdev_remove_map(struct gntdev_priv *priv, struct gntdev_grant_map *map); +#endif + #endif diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c index dc57c6a25525..84cba67c6ad7 100644 --- a/drivers/xen/gntdev-dmabuf.c +++ b/drivers/xen/gntdev-dmabuf.c @@ -3,13 +3,53 @@ /* * Xen dma-buf functionality for gntdev. * + * DMA buffer implementation is based on drivers/gpu/drm/drm_prime.c. + * * Copyright (c) 2018 Oleksandr Andrushchenko, EPAM Systems Inc. */ +#include #include +#include +#include + +#include "gntdev-common.h" #include "gntdev-dmabuf.h" +struct gntdev_dmabuf { + struct gntdev_dmabuf_priv *priv; + struct dma_buf *dmabuf; + struct list_head next; + int fd; + + union { + struct { + /* Exported buffers are reference counted. */ + struct kref refcount; + + struct gntdev_priv *priv; + struct gntdev_grant_map *map; + } exp; + } u; + + /* Number of pages this buffer has. */ + int nr_pages; + /* Pages of this buffer. */ + struct page **pages; +}; + +struct gntdev_dmabuf_wait_obj { + struct list_head next; + struct gntdev_dmabuf *gntdev_dmabuf; + struct completion completion; +}; + +struct gntdev_dmabuf_attachment { + struct sg_table *sgt; + enum dma_data_direction dir; +}; + struct gntdev_dmabuf_priv { /* List of exported DMA buffers. */ struct list_head exp_list; @@ -23,17 +63,439 @@ struct gntdev_dmabuf_priv { /* Implementation of wait for exported DMA buffer to be released. */ +static void dmabuf_exp_release(struct kref *kref); + +static struct gntdev_dmabuf_wait_obj * +dmabuf_exp_wait_obj_new(struct gntdev_dmabuf_priv *priv, + struct gntdev_dmabuf *gntdev_dmabuf) +{ + struct gntdev_dmabuf_wait_obj *obj; + + obj = kzalloc(sizeof(*obj), GFP_KERNEL); + if (!obj) + return ERR_PTR(-ENOMEM); + + init_completion(&obj->completion); + obj->gntdev_dmabuf = gntdev_dmabuf; + + mutex_lock(&priv->lock); + list_add(&obj->next, &priv->exp_wait_list); + /* Put our reference and wait for gntdev_dmabuf's release to fire. */ + kref_put(&gntdev_dmabuf->u.exp.refcount, dmabuf_exp_release); + mutex_unlock(&priv->lock); + return obj; +} + +static void dmabuf_exp_wait_obj_free(struct gntdev_dmabuf_priv *priv, +struct gntdev_dmabuf_wait_obj *obj) +{ + struct gntdev_dmabuf_wait_obj *cur_obj, *q; + + mutex_lock(&priv->lock); + list_for_each_entry_safe(cur_obj, q, &priv->exp_wait_list, next) + if (cur_obj == obj) { + list_del(&obj->next); + kfree(obj); + break; + } + mutex_unlock(&priv->lock); Do we really need to walk the list? And if we do, do we need the safe variant of the walk? We are holding the lock. Here and elsewhere. +} + +static int dmabuf_exp_wait_obj_wait(struct gntdev_dmabuf_wait_obj *obj, + u32 wait_to_ms) +{ + if (wait_for_completion_timeout(&obj->completion, + msecs_to_jiffies(wait_to_ms)) <= 0) + return -ETIMEDOUT; + + return 0; +} + +static void dmabuf_exp_wait_obj_signal(struct gntdev_dmabuf_priv
[PATCH v2 21/27] drm/sun4i: DW HDMI PHY: Add support for second PLL
Some DW HDMI PHYs, like those found in A64 and R40 SoCs, can select between two clock parents. Add code which reads second PLL from DT. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 ++ drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 20 +++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 3ba71aff92fc..46a3aa6a53a9 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h @@ -147,6 +147,7 @@ struct sun8i_hdmi_phy; struct sun8i_hdmi_phy_variant { bool has_phy_clk; + bool has_second_pll; void (*phy_init)(struct sun8i_hdmi_phy *phy); void (*phy_disable)(struct dw_hdmi *hdmi, struct sun8i_hdmi_phy *phy); @@ -160,6 +161,7 @@ struct sun8i_hdmi_phy { struct clk *clk_mod; struct clk *clk_phy; struct clk *clk_pll0; + struct clk *clk_pll1; unsigned intrcal; struct regmap *regs; struct reset_control*rst_phy; diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index cd07ceb71601..f50072ae054a 100644 --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c @@ -482,10 +482,19 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, struct device_node *node) goto err_put_clk_mod; } + if (phy->variant->has_second_pll) { + phy->clk_pll1 = of_clk_get_by_name(node, "pll-1"); + if (IS_ERR(phy->clk_pll1)) { + dev_err(dev, "Could not get pll-1 clock\n"); + ret = PTR_ERR(phy->clk_pll1); + goto err_put_clk_pll0; + } + } + ret = sun8i_phy_clk_create(phy, dev); if (ret) { dev_err(dev, "Couldn't create the PHY clock\n"); - goto err_put_clk_pll0; + goto err_put_clk_pll1; } clk_prepare_enable(phy->clk_phy); @@ -528,9 +537,10 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, struct device_node *node) reset_control_put(phy->rst_phy); err_disable_clk_phy: clk_disable_unprepare(phy->clk_phy); +err_put_clk_pll1: + clk_put(phy->clk_pll1); err_put_clk_pll0: - if (phy->variant->has_phy_clk) - clk_put(phy->clk_pll0); + clk_put(phy->clk_pll0); err_put_clk_mod: clk_put(phy->clk_mod); err_put_clk_bus: @@ -551,8 +561,8 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi *hdmi) reset_control_put(phy->rst_phy); - if (phy->variant->has_phy_clk) - clk_put(phy->clk_pll0); + clk_put(phy->clk_pll0); + clk_put(phy->clk_pll1); clk_put(phy->clk_mod); clk_put(phy->clk_bus); } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 10/27] dt-bindings: display: sun4i-drm: Add R40 TV TCON description
TCON description is expanded with R40 TV TCON compatibles. TV TCONs, which are connected to TCON TOP muxes, such as those on R40 SoC, also needs additional clock gate to be specified. Signed-off-by: Jernej Skrabec --- .../devicetree/bindings/display/sunxi/sun4i-drm.txt | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index ef64c589a4b3..68c4b2995624 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt @@ -145,6 +145,7 @@ Required properties: * allwinner,sun8i-a33-tcon * allwinner,sun8i-a83t-tcon-lcd * allwinner,sun8i-a83t-tcon-tv + * allwinner,sun8i-r40-tcon-tv * allwinner,sun8i-v3s-tcon * allwinner,sun9i-a80-tcon-lcd * allwinner,sun9i-a80-tcon-tv @@ -178,8 +179,10 @@ For TCONs with channel 0, there is one more clock required: - 'tcon-ch0': The clock driving the TCON channel 0 For TCONs with channel 1, there is one more clock required: - 'tcon-ch1': The clock driving the TCON channel 1 +TV TCONs which are connected to TCON TOP (found in R40 SoC) need one more clock: + - 'tcon-top': TV TCON gate found in TCON TOP unit -When TCON support LVDS (all TCONs except TV TCON on A83T and those found +When TCON support LVDS (all TCONs except TV TCONs on A83T, R40 and those found in A13, H3, H5 and V3s SoCs), you need one more reset line: - 'lvds': The reset line driving the LVDS logic -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 22/27] drm/sun4i: Add support for second clock parent to DW HDMI PHY clk driver
Expand HDMI PHY clock driver to support second clock parent. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 4 +- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 3 +- drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 90 -- 3 files changed, 73 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 46a3aa6a53a9..aadbe0a10b0c 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h @@ -99,6 +99,7 @@ #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_ENBIT(28) #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27) #define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26) +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26 #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25) #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x)((x) << 22) #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20) @@ -190,6 +191,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi *hdmi); void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy); const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void); -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev); +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev, +bool second_parent); #endif /* _SUN8I_DW_HDMI_H_ */ diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index f50072ae054a..e1b7196d4587 100644 --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c @@ -491,7 +491,8 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, struct device_node *node) } } - ret = sun8i_phy_clk_create(phy, dev); + ret = sun8i_phy_clk_create(phy, dev, + phy->variant->has_second_pll); if (ret) { dev_err(dev, "Couldn't create the PHY clock\n"); goto err_put_clk_pll1; diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c index faea449812f8..a4d31fe3abff 100644 --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c @@ -22,35 +22,45 @@ static int sun8i_phy_clk_determine_rate(struct clk_hw *hw, { unsigned long rate = req->rate; unsigned long best_rate = 0; + struct clk_hw *best_parent = NULL; struct clk_hw *parent; int best_div = 1; - int i; + int i, p; - parent = clk_hw_get_parent(hw); - - for (i = 1; i <= 16; i++) { - unsigned long ideal = rate * i; - unsigned long rounded; - - rounded = clk_hw_round_rate(parent, ideal); + for (p = 0; p < clk_hw_get_num_parents(hw); p++) { + parent = clk_hw_get_parent_by_index(hw, p); + if (!parent) + continue; - if (rounded == ideal) { - best_rate = rounded; - best_div = i; - break; + for (i = 1; i <= 16; i++) { + unsigned long ideal = rate * i; + unsigned long rounded; + + rounded = clk_hw_round_rate(parent, ideal); + + if (rounded == ideal) { + best_rate = rounded; + best_div = i; + best_parent = parent; + break; + } + + if (!best_rate || + abs(rate - rounded / i) < + abs(rate - best_rate / best_div)) { + best_rate = rounded; + best_div = i; + best_parent = parent; + } } - if (!best_rate || - abs(rate - rounded / i) < - abs(rate - best_rate / best_div)) { - best_rate = rounded; - best_div = i; - } + if (best_rate / best_div == rate) + break; } req->rate = best_rate / best_div; req->best_parent_rate = best_rate; - req->best_parent_hw = parent; + req->best_parent_hw = best_parent; return 0; } @@ -95,22 +105,58 @@ static int sun8i_phy_clk_set_rate(struct clk_hw *hw, unsigned long rate, return 0; } +static u8 sun8i_phy_clk_get_parent(struct clk_hw *hw) +{ + struct sun8i_phy_clk *priv = hw_to_phy_clk(hw); + u32 reg; + + regmap_read(priv->phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, ®); + reg = (reg & SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK) >> + SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT; + +
[PATCH v2 03/27] clk: sunxi-ng: r40: Export video PLLs
Video PLLs need to be referenced in R40 DT as possible HDMI PHY parent. Export them. Reviewed-by: Rob Herring Signed-off-by: Jernej Skrabec --- drivers/clk/sunxi-ng/ccu-sun8i-r40.h | 8 ++-- include/dt-bindings/clock/sun8i-r40-ccu.h | 4 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-r40.h b/drivers/clk/sunxi-ng/ccu-sun8i-r40.h index 0db8e1e97af8..db2a1243f9ff 100644 --- a/drivers/clk/sunxi-ng/ccu-sun8i-r40.h +++ b/drivers/clk/sunxi-ng/ccu-sun8i-r40.h @@ -25,7 +25,9 @@ #define CLK_PLL_AUDIO_2X 4 #define CLK_PLL_AUDIO_4X 5 #define CLK_PLL_AUDIO_8X 6 -#define CLK_PLL_VIDEO0 7 + +/* PLL_VIDEO0 is exported */ + #define CLK_PLL_VIDEO0_2X 8 #define CLK_PLL_VE 9 #define CLK_PLL_DDR0 10 @@ -34,7 +36,9 @@ #define CLK_PLL_PERIPH0_2X 13 #define CLK_PLL_PERIPH114 #define CLK_PLL_PERIPH1_2X 15 -#define CLK_PLL_VIDEO1 16 + +/* PLL_VIDEO1 is exported */ + #define CLK_PLL_VIDEO1_2X 17 #define CLK_PLL_SATA 18 #define CLK_PLL_SATA_OUT 19 diff --git a/include/dt-bindings/clock/sun8i-r40-ccu.h b/include/dt-bindings/clock/sun8i-r40-ccu.h index 4fa5f69fc297..f9e15a235626 100644 --- a/include/dt-bindings/clock/sun8i-r40-ccu.h +++ b/include/dt-bindings/clock/sun8i-r40-ccu.h @@ -43,6 +43,10 @@ #ifndef _DT_BINDINGS_CLK_SUN8I_R40_H_ #define _DT_BINDINGS_CLK_SUN8I_R40_H_ +#define CLK_PLL_VIDEO0 7 + +#define CLK_PLL_VIDEO1 16 + #define CLK_CPU24 #define CLK_BUS_MIPI_DSI 29 -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] drm/rockchip: vop: fix irq disabled after vop driver probed
Hi Heiko, On 12/06/18 13:15, Heiko Stuebner wrote: > From: Sandy Huang > > The vop irq is shared between vop and iommu and irq probing in the > iommu driver moved to the probe function recently. This can in some > cases lead to a stall if the irq is triggered while the vop driver > still has it disabled, but the vop irq handler gets called. > > But there is no real need to disable the irq, as the vop can simply > also track its enabled state and ignore irqs in that case. > For this we can simply check the power-domain state of the vop, > similar to how the iommu driver does it. > > So remove the enable/disable handling and add appropriate condition > to the irq handler. > > changes in v2: > - move to just check the power-domain state > - add clock handling > changes in v3: > - clarify comment to speak of runtime-pm not power-domain > > Fixes: d0b912bd4c23 ("iommu/rockchip: Request irqs in rk_iommu_probe()") > Cc: sta...@vger.kernel.org > Signed-off-by: Sandy Huang > Signed-off-by: Heiko Stuebner > Tested-by: Ezequiel Garcia > --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 28 ++--- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 9a1f272e41c7..ae8a69793aed 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -573,8 +573,6 @@ static int vop_enable(struct drm_crtc *crtc) > > spin_unlock(&vop->reg_lock); > > - enable_irq(vop->irq); > - > drm_crtc_vblank_on(crtc); > > return 0; > @@ -618,8 +616,6 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc, > > vop_dsp_hold_valid_irq_disable(vop); > > - disable_irq(vop->irq); > - > vop->is_enabled = false; > > /* > @@ -1195,6 +1191,16 @@ static irqreturn_t vop_isr(int irq, void *data) > uint32_t active_irqs; > int ret = IRQ_NONE; > > + /* > + * The irq is shared with the iommu. If the runtime-pm state of the > + * vop-device is disabled the irq has to be targetted at the iommu. > + */ > + if (!pm_runtime_get_if_in_use(vop->dev)) > + return IRQ_NONE; > + > + if (WARN_ON(vop_core_clks_enable(vop))) > + goto out; As I mentioned before, a WARN_ON() in an interrupt handler is a good way to make a bad problem even worse, and will give information (full register and stack dump) that is mostly useless to the context at hand. Turning it to a dev_warn_ratelimited() (or DRM_ERROR_RATELIMITED if you want to be DRM compliant) would be a better approach, IMHO. > + > /* >* interrupt register has interrupt status, enable and clear bits, we >* must hold irq_lock to avoid a race with enable/disable_vblank(). > @@ -1209,8 +1215,11 @@ static irqreturn_t vop_isr(int irq, void *data) > spin_unlock(&vop->irq_lock); > > /* This is expected for vop iommu irqs, since the irq is shared */ > - if (!active_irqs) > - return IRQ_NONE; > + if (!active_irqs) { > + ret = IRQ_NONE; > + vop_core_clks_disable(vop); > + goto out; > + } A couple of nits: ret is already set to IRQ_NONE at this stage, and you could simply rewrite it as: if (!active_irq) goto out_disable; > > if (active_irqs & DSP_HOLD_VALID_INTR) { > complete(&vop->dsp_hold_completion); > @@ -1236,6 +1245,10 @@ static irqreturn_t vop_isr(int irq, void *data) > DRM_DEV_ERROR(vop->dev, "Unknown VOP IRQs: %#02x\n", > active_irqs); > with the "out_disable" label placed here. > + vop_core_clks_disable(vop); > + > +out: > + pm_runtime_put(vop->dev); > return ret; > } > > @@ -1614,9 +1627,6 @@ static int vop_bind(struct device *dev, struct device > *master, void *data) > if (ret) > goto err_disable_pm_runtime; > > - /* IRQ is initially disabled; it gets enabled in power_on */ > - disable_irq(vop->irq); > - > return 0; > > err_disable_pm_runtime: > Thanks, M. -- Jazz is not dead. It just smells funny... ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 12/27] drm/sun4i: tcon: Generalize engine search algorithm
Current "old" method to find engine worked pretty well for DE2. However, it doesn't work when TCON TOP is between mixer (engine) and TCON. TCON TOP has multiple input ports, but current engine search algorithm expects only one. This can be fixed by first looking for output port id and selecting matching input by subtracting 1 for the next round. This work even if there is only one input and output. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 0afb5a94a414..5dae623968f7 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -802,12 +802,14 @@ static int sun4i_tcon_init_regmap(struct device *dev, */ static struct sunxi_engine * sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, - struct device_node *node) + struct device_node *node, + u32 port_id) { struct device_node *port, *ep, *remote; struct sunxi_engine *engine = ERR_PTR(-EINVAL); + u32 reg = 0; - port = of_graph_get_port_by_id(node, 0); + port = of_graph_get_port_by_id(node, port_id); if (!port) return ERR_PTR(-EINVAL); @@ -837,8 +839,20 @@ sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, if (remote == engine->node) goto out_put_remote; + /* +* According to device tree binding input ports have even id +* number and output ports have odd id. Since component with +* more than one input and one output (TCON TOP) exits, correct +* remote input id has to be calculated by subtracting 1 from +* remote output id. If this for some reason can't be done, 0 +* is used as input port id. +*/ + port = of_graph_get_remote_port(ep); + if (!of_property_read_u32(port, "reg", ®) && reg > 0) + reg -= 1; + /* keep looking through upstream ports */ - engine = sun4i_tcon_find_engine_traverse(drv, remote); + engine = sun4i_tcon_find_engine_traverse(drv, remote, reg); out_put_remote: of_node_put(remote); @@ -961,7 +975,7 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv, /* Fallback to old method by traversing input endpoints */ of_node_put(port); - return sun4i_tcon_find_engine_traverse(drv, node); + return sun4i_tcon_find_engine_traverse(drv, node, 0); } static int sun4i_tcon_bind(struct device *dev, struct device *master, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 06/27] drm/sun4i: Fix releasing node when enumerating enpoints
sun4i_drv_add_endpoints() has a memory leak since it uses of_node_put() when remote is equal to NULL and does nothing when remote has a valid pointer. Invert the logic to fix memory leak. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun4i_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index 50d19605c38f..e15fa2389e3f 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -283,7 +283,6 @@ static int sun4i_drv_add_endpoints(struct device *dev, remote = of_graph_get_remote_port_parent(ep); if (!remote) { DRM_DEBUG_DRIVER("Error retrieving the output node\n"); - of_node_put(remote); continue; } @@ -297,11 +296,13 @@ static int sun4i_drv_add_endpoints(struct device *dev, if (of_graph_parse_endpoint(ep, &endpoint)) { DRM_DEBUG_DRIVER("Couldn't parse endpoint\n"); + of_node_put(remote); continue; } if (!endpoint.id) { DRM_DEBUG_DRIVER("Endpoint is our panel... skipping\n"); + of_node_put(remote); continue; } } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 15/27] drm/sun4i: Add support for R40 TV TCON
R40 TV TCON is similar to the A83T TV TCON, except that it needs additional gate to be enabled. Add support for it. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index c9ffa5381185..f20da2aa2165 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1326,6 +1326,11 @@ static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = { .has_channel_1 = true, }; +static const struct sun4i_tcon_quirks sun8i_r40_tv_quirks = { + .has_channel_1 = true, + .has_tcon_top_gate = true, +}; + static const struct sun4i_tcon_quirks sun8i_v3s_quirks = { .has_channel_0 = true, }; @@ -1350,6 +1355,7 @@ const struct of_device_id sun4i_tcon_of_table[] = { { .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks }, { .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = &sun8i_a83t_lcd_quirks }, { .compatible = "allwinner,sun8i-a83t-tcon-tv", .data = &sun8i_a83t_tv_quirks }, + { .compatible = "allwinner,sun8i-r40-tcon-tv", .data = &sun8i_r40_tv_quirks }, { .compatible = "allwinner,sun8i-v3s-tcon", .data = &sun8i_v3s_quirks }, { .compatible = "allwinner,sun9i-a80-tcon-lcd", .data = &sun9i_a80_tcon_lcd_quirks }, { .compatible = "allwinner,sun9i-a80-tcon-tv", .data = &sun9i_a80_tcon_tv_quirks }, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 04/27] dt-bindings: display: sunxi-drm: Add TCON TOP description
TCON TOP main purpose is to configure whole display pipeline. It determines relationships between mixers and TCONs, selects source TCON for HDMI, muxes LCD and TV encoder GPIO output, selects TV encoder clock source and contains additional TV TCON and DSI gates. Signed-off-by: Jernej Skrabec --- .../bindings/display/sunxi/sun4i-drm.txt | 45 +++ include/dt-bindings/clock/sun8i-tcon-top.h| 11 + 2 files changed, 56 insertions(+) create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index 3346c1e2a7a0..ef64c589a4b3 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt @@ -187,6 +187,51 @@ And on the A23, A31, A31s and A33, you need one more clock line: - 'lvds-alt': An alternative clock source, separate from the TCON channel 0 clock, that can be used to drive the LVDS clock +TCON TOP + + +TCON TOPs main purpose is to configure whole display pipeline. It determines +relationships between mixers and TCONs, selects source TCON for HDMI, muxes +LCD and TV encoder GPIO output, selects TV encoder clock source and contains +additional TV TCON and DSI gates. + +It allows display pipeline to be configured in very different ways: + + / LCD0/LVDS0 + / TCON-LCD0 + | \ MIPI DSI + mixer0 | +\/ TCON-LCD1 - LCD1/LVDS1 + TCON-TOP +/\ TCON-TV0 - TVE0/RGB + mixer1 | \ + | TCON-TOP - HDMI + | / + \ TCON-TV1 - TVE1/RGB + +Note that both TCON TOP references same physical unit. + +Required properties: + - compatible: value must be one of: +* allwinner,sun8i-r40-tcon-top + - reg: base address and size of the memory-mapped region. + - clocks: phandle to the clocks feeding the TCON TOP +* bus: TCON TOP interface clock + - clock-names: clock name mentioned above + - resets: phandle to the reset line driving the DRC +* rst: TCON TOP reset line + - reset-names: reset name mentioned above + - #clock-cells : must contain 1 + +- ports: A ports node with endpoint definitions as defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. The first port +should be the input for mixer0 mux. The second should be the output for that +mux. Third port should be input for mixer1 mux. Fourth port should be output +for mixer1 mux. Fifth port should be input for HDMI mux. Sixth port should +be output for it. All output endpoints should have reg property with the id +of the target TCON. All ports should have only one enpoint connected to +remote endpoint. + DRC --- diff --git a/include/dt-bindings/clock/sun8i-tcon-top.h b/include/dt-bindings/clock/sun8i-tcon-top.h new file mode 100644 index ..c05e92770402 --- /dev/null +++ b/include/dt-bindings/clock/sun8i-tcon-top.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ +/* Copyright (C) 2018 Jernej Skrabec */ + +#ifndef _DT_BINDINGS_CLOCK_SUN8I_TCON_TOP_H_ +#define _DT_BINDINGS_CLOCK_SUN8I_TCON_TOP_H_ + +#define CLK_BUS_TCON_TOP_DSI 0 +#define CLK_BUS_TCON_TOP_TV0 1 +#define CLK_BUS_TCON_TOP_TV1 2 + +#endif /* _DT_BINDINGS_CLOCK_SUN8I_TCON_TOP_H_ */ -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 07/27] drm/sun4i: Split out code for enumerating endpoints in output port
Until now, each node has one input port and one output port. However, with TCON TOP this is no longer true. It has 3 input and 3 output ports. In order to prepare to this situation, split out the code which checks all endpoints in input port and adds available components to fifo. This patch doesn't do any functional change. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun4i_drv.c | 84 +-- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index e15fa2389e3f..20193d6f33ba 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -231,12 +231,55 @@ struct endpoint_list { DECLARE_KFIFO(fifo, struct device_node *, 16); }; +static void sun4i_drv_traverse_endpoints(struct endpoint_list *list, +struct device_node *node, +int port_id) +{ + struct device_node *ep, *remote, *port; + + port = of_graph_get_port_by_id(node, port_id); + if (!port) { + DRM_DEBUG_DRIVER("No output to bind on port %d\n", port_id); + return; + } + + for_each_available_child_of_node(port, ep) { + remote = of_graph_get_remote_port_parent(ep); + if (!remote) { + DRM_DEBUG_DRIVER("Error retrieving the output node\n"); + continue; + } + + /* +* If the node is our TCON, the first port is used for +* panel or bridges, and will not be part of the +* component framework. +*/ + if (sun4i_drv_node_is_tcon(node)) { + struct of_endpoint endpoint; + + if (of_graph_parse_endpoint(ep, &endpoint)) { + DRM_DEBUG_DRIVER("Couldn't parse endpoint\n"); + of_node_put(remote); + continue; + } + + if (!endpoint.id) { + DRM_DEBUG_DRIVER("Endpoint is our panel... skipping\n"); + of_node_put(remote); + continue; + } + } + + kfifo_put(&list->fifo, remote); + } +} + static int sun4i_drv_add_endpoints(struct device *dev, struct endpoint_list *list, struct component_match **match, struct device_node *node) { - struct device_node *port, *ep, *remote; int count = 0; /* @@ -272,43 +315,8 @@ static int sun4i_drv_add_endpoints(struct device *dev, count++; } - /* Inputs are listed first, then outputs */ - port = of_graph_get_port_by_id(node, 1); - if (!port) { - DRM_DEBUG_DRIVER("No output to bind\n"); - return count; - } - - for_each_available_child_of_node(port, ep) { - remote = of_graph_get_remote_port_parent(ep); - if (!remote) { - DRM_DEBUG_DRIVER("Error retrieving the output node\n"); - continue; - } - - /* -* If the node is our TCON, the first port is used for -* panel or bridges, and will not be part of the -* component framework. -*/ - if (sun4i_drv_node_is_tcon(node)) { - struct of_endpoint endpoint; - - if (of_graph_parse_endpoint(ep, &endpoint)) { - DRM_DEBUG_DRIVER("Couldn't parse endpoint\n"); - of_node_put(remote); - continue; - } - - if (!endpoint.id) { - DRM_DEBUG_DRIVER("Endpoint is our panel... skipping\n"); - of_node_put(remote); - continue; - } - } - - kfifo_put(&list->fifo, remote); - } + /* each node has at least one output */ + sun4i_drv_traverse_endpoints(list, node, 1); return count; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 02/27] clk: sunxi-ng: r40: Allow setting parent rate to display related clocks
Display related peripherals need precise clocks to operate correctly. Allow DE2, TCONs and HDMI to set parent clock. Signed-off-by: Jernej Skrabec --- drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c index c16a62a7bdbd..fa5317719684 100644 --- a/drivers/clk/sunxi-ng/ccu-sun8i-r40.c +++ b/drivers/clk/sunxi-ng/ccu-sun8i-r40.c @@ -655,7 +655,8 @@ static SUNXI_CCU_GATE(dram_deinterlace_clk, "dram-deinterlace", "dram", static const char * const de_parents[] = { "pll-periph0-2x", "pll-de" }; static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents, -0x104, 0, 4, 24, 3, BIT(31), 0); +0x104, 0, 4, 24, 3, BIT(31), +CLK_SET_RATE_PARENT); static SUNXI_CCU_M_WITH_MUX_GATE(mp_clk, "mp", de_parents, 0x108, 0, 4, 24, 3, BIT(31), 0); @@ -667,9 +668,11 @@ static SUNXI_CCU_MUX_WITH_GATE(tcon_lcd0_clk, "tcon-lcd0", tcon_parents, static SUNXI_CCU_MUX_WITH_GATE(tcon_lcd1_clk, "tcon-lcd1", tcon_parents, 0x114, 24, 3, BIT(31), CLK_SET_RATE_PARENT); static SUNXI_CCU_M_WITH_MUX_GATE(tcon_tv0_clk, "tcon-tv0", tcon_parents, -0x118, 0, 4, 24, 3, BIT(31), 0); +0x118, 0, 4, 24, 3, BIT(31), +CLK_SET_RATE_PARENT); static SUNXI_CCU_M_WITH_MUX_GATE(tcon_tv1_clk, "tcon-tv1", tcon_parents, -0x11c, 0, 4, 24, 3, BIT(31), 0); +0x11c, 0, 4, 24, 3, BIT(31), +CLK_SET_RATE_PARENT); static const char * const deinterlace_parents[] = { "pll-periph0", "pll-periph1" }; @@ -699,7 +702,8 @@ static SUNXI_CCU_GATE(avs_clk, "avs", "osc24M", static const char * const hdmi_parents[] = { "pll-video0", "pll-video1" }; static SUNXI_CCU_M_WITH_MUX_GATE(hdmi_clk, "hdmi", hdmi_parents, -0x150, 0, 4, 24, 2, BIT(31), 0); +0x150, 0, 4, 24, 2, BIT(31), +CLK_SET_RATE_PARENT); static SUNXI_CCU_GATE(hdmi_slow_clk, "hdmi-slow","osc24M", 0x154, BIT(31), 0); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/9] xen/balloon: Share common memory reservation routines
On 06/13/2018 03:47 AM, Boris Ostrovsky wrote: On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko diff --git a/include/xen/mem-reservation.h b/include/xen/mem-reservation.h new file mode 100644 index ..e0939387278d --- /dev/null +++ b/include/xen/mem-reservation.h @@ -0,0 +1,64 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * Xen memory reservation utilities. + * + * Copyright (c) 2003, B Dragovic + * Copyright (c) 2003-2004, M Williamson, K Fraser + * Copyright (c) 2005 Dan M. Smith, IBM Corporation + * Copyright (c) 2010 Daniel Kiper + * Copyright (c) 2018 Oleksandr Andrushchenko, EPAM Systems Inc. + */ + +#ifndef _XENMEM_RESERVATION_H +#define _XENMEM_RESERVATION_H + +#include +#include + +#include +#include + +#include +#include I should have noticed this in the previous post but I suspect most of these includes belong in the C file. For example, there is no reason for hypercall.h here. Yes, it seems that the header can only have #include Will move the rest into the .c file -boris + +static inline void xenmem_reservation_scrub_page(struct page *page) +{ +#ifdef CONFIG_XEN_SCRUB_PAGES + clear_highpage(page); +#endif +} + +#ifdef CONFIG_XEN_HAVE_PVMMU +void __xenmem_reservation_va_mapping_update(unsigned long count, + struct page **pages, + xen_pfn_t *frames); + +void __xenmem_reservation_va_mapping_reset(unsigned long count, + struct page **pages); +#endif + +static inline void xenmem_reservation_va_mapping_update(unsigned long count, + struct page **pages, + xen_pfn_t *frames) +{ +#ifdef CONFIG_XEN_HAVE_PVMMU + if (!xen_feature(XENFEAT_auto_translated_physmap)) + __xenmem_reservation_va_mapping_update(count, pages, frames); +#endif +} + +static inline void xenmem_reservation_va_mapping_reset(unsigned long count, + struct page **pages) +{ +#ifdef CONFIG_XEN_HAVE_PVMMU + if (!xen_feature(XENFEAT_auto_translated_physmap)) + __xenmem_reservation_va_mapping_reset(count, pages); +#endif +} + +int xenmem_reservation_increase(int count, xen_pfn_t *frames); + +int xenmem_reservation_decrease(int count, xen_pfn_t *frames); + +#endif ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 7/9] xen/gntdev: Add initial support for dma-buf UAPI
On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index a09db23e9663..e82660d81d7e 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -48,6 +48,9 @@ #include #include "gntdev-common.h" +#ifdef CONFIG_XEN_GNTDEV_DMABUF +#include "gntdev-dmabuf.h" +#endif MODULE_LICENSE("GPL"); MODULE_AUTHOR("Derek G. Murray , " @@ -566,6 +569,15 @@ static int gntdev_open(struct inode *inode, struct file *flip) INIT_LIST_HEAD(&priv->freeable_maps); mutex_init(&priv->lock); +#ifdef CONFIG_XEN_GNTDEV_DMABUF + priv->dmabuf_priv = gntdev_dmabuf_init(); + if (IS_ERR(priv->dmabuf_priv)) { + ret = PTR_ERR(priv->dmabuf_priv); + kfree(priv); + return ret; + } +#endif + if (use_ptemod) { priv->mm = get_task_mm(current); if (!priv->mm) { @@ -616,8 +628,13 @@ static int gntdev_release(struct inode *inode, struct file *flip) WARN_ON(!list_empty(&priv->freeable_maps)); mutex_unlock(&priv->lock); +#ifdef CONFIG_XEN_GNTDEV_DMABUF + gntdev_dmabuf_fini(priv->dmabuf_priv); +#endif + if (use_ptemod) mmu_notifier_unregister(&priv->mn, priv->mm); + kfree(priv); return 0; } @@ -987,6 +1004,107 @@ static long gntdev_ioctl_grant_copy(struct gntdev_priv *priv, void __user *u) return ret; } +#ifdef CONFIG_XEN_GNTDEV_DMABUF +static long +gntdev_ioctl_dmabuf_exp_from_refs(struct gntdev_priv *priv, + struct ioctl_gntdev_dmabuf_exp_from_refs __user *u) Didn't we agree that this code moves to gntdev-dmabuf.c ? -boris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 00/27] Add support for R40 HDMI pipeline
This series adds support for R40 HDMI pipeline. It is a bit special than other already supported pipelines because it has additional unit called TCON TOP responsible for relationship configuration between mixers, TCONs and HDMI. Additionally, it has additional gates for DSI and TV TCONs, TV encoder clock settings and pin muxing between LCD and TV encoders. However, it seems that TCON TOP will become a norm, since newer Allwinner SoCs like H6 also have this unit. I tested different possible configurations: - mixer0 <> TCON-TV0 <> HDMI - mixer0 <> TCON-TV1 <> HDMI - mixer1 <> TCON-TV0 <> HDMI - mixer1 <> TCON-TV1 <> HDMI Please review. Best regards, Jernej Changes from v1: - Split DT bindings patch and updated description - Split HDMI PHY patch - Move header file from TCON TOP patch to dt bindings patch - Added Rob reviewed-by tag - Used clk_hw_register_gate() instead of custom gate registration code - Reworked TCON TOP to be part of of-graph. Because of that, a lot of new patches were added. - Droped mixer index quirk patch - Reworked TCON support for TCON TOP - Updated commit messages Jernej Skrabec (27): clk: sunxi-ng: r40: Add minimal rate for video PLLs clk: sunxi-ng: r40: Allow setting parent rate to display related clocks clk: sunxi-ng: r40: Export video PLLs dt-bindings: display: sunxi-drm: Add TCON TOP description drm/sun4i: Add TCON TOP driver drm/sun4i: Fix releasing node when enumerating enpoints drm/sun4i: Split out code for enumerating endpoints in output port drm/sun4i: Add support for traversing graph with TCON TOP drm/sun4i: Don't skip TCONs if they don't have channel 0 dt-bindings: display: sun4i-drm: Add R40 TV TCON description drm/sun4i: tcon: Add support for tcon-top gate drm/sun4i: tcon: Generalize engine search algorithm drm/sun4i: Don't check for LVDS and RGB when TCON has only ch1 drm/sun4i: Don't check for panel or bridge on TV TCONs drm/sun4i: Add support for R40 TV TCON dt-bindings: display: sun4i-drm: Add R40 mixer compatibles drm/sun4i: Add support for R40 mixers dt-bindings: display: sun4i-drm: Add description of A64 HDMI PHY drm/sun4i: Enable DW HDMI PHY clock drm/sun4i: Don't change clock bits in DW HDMI PHY driver drm/sun4i: DW HDMI PHY: Add support for second PLL drm/sun4i: Add support for second clock parent to DW HDMI PHY clk driver drm/sun4i: Add support for A64 HDMI PHY drm: of: Export drm_crtc_port_mask() drm/sun4i: DW HDMI: Expand algorithm for possible crtcs ARM: dts: sun8i: r40: Add HDMI pipeline ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 Ultra .../bindings/display/sunxi/sun4i-drm.txt | 56 +++- .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 45 +++ arch/arm/boot/dts/sun8i-r40.dtsi | 257 ++ drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 58 ++-- drivers/clk/sunxi-ng/ccu-sun8i-r40.h | 8 +- drivers/gpu/drm/drm_of.c | 4 +- drivers/gpu/drm/sun4i/Makefile| 3 +- drivers/gpu/drm/sun4i/sun4i_drv.c | 121 ++--- drivers/gpu/drm/sun4i/sun4i_tcon.c| 83 -- drivers/gpu/drm/sun4i/sun4i_tcon.h| 4 + drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 46 +++- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 8 +- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c| 54 +++- drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c| 90 -- drivers/gpu/drm/sun4i/sun8i_mixer.c | 24 ++ drivers/gpu/drm/sun4i/sun8i_tcon_top.c| 248 + drivers/gpu/drm/sun4i/sun8i_tcon_top.h| 38 +++ include/drm/drm_of.h | 8 + include/dt-bindings/clock/sun8i-r40-ccu.h | 4 + include/dt-bindings/clock/sun8i-tcon-top.h| 11 + 20 files changed, 1047 insertions(+), 123 deletions(-) create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.c create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.h create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 08/27] drm/sun4i: Add support for traversing graph with TCON TOP
TCON TOP is different from other nodes in graph by having 3 input and 3 output ports. Additionally, connection to TV TCON might lead back to HDMI mux input port, creating loops. Add support for traversing such graph. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/sun4i_drv.c | 24 1 file changed, 24 insertions(+) diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index 20193d6f33ba..e6c62c079146 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -26,6 +26,7 @@ #include "sun4i_frontend.h" #include "sun4i_framebuffer.h" #include "sun4i_tcon.h" +#include "sun8i_tcon_top.h" DEFINE_DRM_GEM_CMA_FOPS(sun4i_drv_fops); @@ -197,6 +198,11 @@ static bool sun4i_drv_node_is_tcon(struct device_node *node) return !!of_match_node(sun4i_tcon_of_table, node); } +static bool sun4i_drv_node_is_tcon_top(struct device_node *node) +{ + return !!of_match_node(sun8i_tcon_top_of_table, node); +} + static int compare_of(struct device *dev, void *data) { DRM_DEBUG_DRIVER("Comparing of node %pOF with %pOF\n", @@ -258,6 +264,18 @@ static void sun4i_drv_traverse_endpoints(struct endpoint_list *list, if (sun4i_drv_node_is_tcon(node)) { struct of_endpoint endpoint; + /* +* TCON TOP is always probed before TCON. However, TCON +* points back to TCON TOP when it is source for HDMI. +* We have to skip it here to prevent infinite looping +* between TCON TOP and TCON. +*/ + if (sun4i_drv_node_is_tcon_top(remote)) { + DRM_DEBUG_DRIVER("TCON output endpoint is TCON TOP... skipping\n"); + of_node_put(remote); + continue; + } + if (of_graph_parse_endpoint(ep, &endpoint)) { DRM_DEBUG_DRIVER("Couldn't parse endpoint\n"); of_node_put(remote); @@ -318,6 +336,12 @@ static int sun4i_drv_add_endpoints(struct device *dev, /* each node has at least one output */ sun4i_drv_traverse_endpoints(list, node, 1); + /* TCON TOP has second and third output */ + if (sun4i_drv_node_is_tcon_top(node)) { + sun4i_drv_traverse_endpoints(list, node, 3); + sun4i_drv_traverse_endpoints(list, node, 5); + } + return count; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 16/27] dt-bindings: display: sun4i-drm: Add R40 mixer compatibles
R40 DE2 mixers are similar to those found in A83T, except it needs different clock settings. Add a compatibles for them. Signed-off-by: Jernej Skrabec --- Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index 68c4b2995624..d84df6d808c2 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt @@ -358,6 +358,8 @@ Required properties: * allwinner,sun8i-a83t-de2-mixer-0 * allwinner,sun8i-a83t-de2-mixer-1 * allwinner,sun8i-h3-de2-mixer-0 +* allwinner,sun8i-r40-de2-mixer-0 +* allwinner,sun8i-r40-de2-mixer-1 * allwinner,sun8i-v3s-de2-mixer - reg: base address and size of the memory-mapped region. - clocks: phandles to the clocks feeding the mixer -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/9] xen/balloon: Share common memory reservation routines
On 06/13/2018 04:07 AM, Boris Ostrovsky wrote: On 06/12/2018 09:41 AM, Oleksandr Andrushchenko wrote: One more thing: please add a comment here saying that frames array is array of PFNs (in Xen granularity), which is what XENMEM_populate_physmap requires. And remove (or update to name the actual call you are making) the corresponding comment in increase_reservation(). I will remove corresponding comments from the balloon's {increase|decrease}_reservation and move those into xenmem_reservation{increase|decrease} where they belong now. I will also put a comment close to xenmem_reservation{increase|decrease}: /* @frames is an array of PFNs */ int xenmem_reservation_increase(int count, xen_pfn_t *frames) { [...] } /* @frames is an array of GFNs */ int xenmem_reservation_decrease(int count, xen_pfn_t *frames) { [...] } + +int xenmem_reservation_increase(int count, xen_pfn_t *frames) +{ + struct xen_memory_reservation reservation = { + .address_bits = 0, + .extent_order = EXTENT_ORDER, + .domid = DOMID_SELF + }; + + set_xen_guest_handle(reservation.extent_start, frames); + reservation.nr_extents = count; + return HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation); +} +EXPORT_SYMBOL_GPL(xenmem_reservation_increase); And similarly, here we are requesting GFNs, and update decrease_reservation(). Please see above -boris Thank you, Oleksandr + +int xenmem_reservation_decrease(int count, xen_pfn_t *frames) +{ + struct xen_memory_reservation reservation = { + .address_bits = 0, + .extent_order = EXTENT_ORDER, + .domid = DOMID_SELF + }; + + set_xen_guest_handle(reservation.extent_start, frames); + reservation.nr_extents = count; + return HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 05/27] drm/sun4i: Add TCON TOP driver
As already described in DT binding, TCON TOP is responsible for configuring display pipeline. In this initial driver focus is on HDMI pipeline, so TVE and LCD configuration is not implemented. Implemented features: - HDMI source selection - clock driver (TCON and DSI gating) - connecting mixers and TCONS Something similar also existed in previous SoCs, except that it was part of first TCON. Signed-off-by: Jernej Skrabec --- drivers/gpu/drm/sun4i/Makefile | 3 +- drivers/gpu/drm/sun4i/sun8i_tcon_top.c | 248 + drivers/gpu/drm/sun4i/sun8i_tcon_top.h | 38 3 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.c create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.h diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile index 2589f4acd5ae..09fbfd6304ba 100644 --- a/drivers/gpu/drm/sun4i/Makefile +++ b/drivers/gpu/drm/sun4i/Makefile @@ -16,7 +16,8 @@ sun8i-drm-hdmi-y += sun8i_hdmi_phy_clk.o sun8i-mixer-y += sun8i_mixer.o sun8i_ui_layer.o \ sun8i_vi_layer.o sun8i_ui_scaler.o \ - sun8i_vi_scaler.o sun8i_csc.o + sun8i_vi_scaler.o sun8i_csc.o \ + sun8i_tcon_top.o sun4i-tcon-y += sun4i_crtc.o sun4i-tcon-y += sun4i_dotclock.o diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c new file mode 100644 index ..60b17e893f08 --- /dev/null +++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Copyright (c) 2018 Jernej Skrabec */ + +#include + +#include + +#include +#include +#include +#include +#include +#include + +#include "sun8i_tcon_top.h" + +struct sun8i_tcon_top_gate { + const char *name; + u8 bit; + int index; +}; + +static const struct sun8i_tcon_top_gate gates[] = { + {"bus-tcon-top-dsi", TCON_TOP_TCON_DSI_GATE, CLK_BUS_TCON_TOP_DSI}, + {"bus-tcon-top-tv0", TCON_TOP_TCON_TV0_GATE, CLK_BUS_TCON_TOP_TV0}, + {"bus-tcon-top-tv1", TCON_TOP_TCON_TV1_GATE, CLK_BUS_TCON_TOP_TV1}, +}; + +static int sun8i_tcon_top_get_connected_ep_id(struct device_node *node, + int port_id) +{ + struct device_node *ep, *remote, *port; + struct of_endpoint endpoint; + + port = of_graph_get_port_by_id(node, port_id); + if (!port) + return -ENOENT; + + for_each_available_child_of_node(port, ep) { + remote = of_graph_get_remote_port_parent(ep); + if (!remote) + continue; + + if (of_device_is_available(remote)) { + of_graph_parse_endpoint(ep, &endpoint); + + of_node_put(remote); + + return endpoint.id; + } + + of_node_put(remote); + } + + return -ENOENT; +} + +static int sun8i_tcon_top_bind(struct device *dev, struct device *master, + void *data) +{ + struct platform_device *pdev = to_platform_device(dev); + struct clk_hw_onecell_data *clk_data; + struct sun8i_tcon_top *tcon_top; + bool mixer0_unused = false; + struct resource *res; + void __iomem *regs; + const char *parent; + int ret, i, id; + u32 val; + + tcon_top = devm_kzalloc(dev, sizeof(*tcon_top), GFP_KERNEL); + if (!tcon_top) + return -ENOMEM; + + clk_data = devm_kzalloc(dev, sizeof(*clk_data) + + sizeof(*clk_data->hws) * CLK_NUM, + GFP_KERNEL); + if (!clk_data) + return -ENOMEM; + tcon_top->clk_data = clk_data; + + spin_lock_init(&tcon_top->reg_lock); + + tcon_top->rst = devm_reset_control_get(dev, "rst"); + if (IS_ERR(tcon_top->rst)) { + dev_err(dev, "Couldn't get our reset line\n"); + return PTR_ERR(tcon_top->rst); + } + + tcon_top->bus = devm_clk_get(dev, "bus"); + if (IS_ERR(tcon_top->bus)) { + dev_err(dev, "Couldn't get the bus clock\n"); + return PTR_ERR(tcon_top->bus); + } + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + regs = devm_ioremap_resource(dev, res); + if (IS_ERR(regs)) + return PTR_ERR(regs); + + ret = reset_control_deassert(tcon_top->rst); + if (ret) { + dev_err(dev, "Could not deassert ctrl reset control\n"); + return ret; + } + + ret = clk_prepare_enable(tcon_top->bus); + if (ret) { + dev_err(dev, "Could not enable bus clock\n"); + goto err_assert_reset; + } + + val = 0; + +