Re: [PATCH 2/5] Fonts: Make font size unsigned in font_desc
On Tue, Oct 27, 2020 at 07:50:58PM +0100, Daniel Vetter wrote: > On Tue, Oct 27, 2020 at 12:33:05PM -0400, Peilin Ye wrote: > > It is improper to define `width` and `height` as signed in `struct > > font_desc`. Make them unsigned. Also, change the corresponding printk() > > format identifiers from `%d` to `%u`, in sti_select_fbfont(). > > > > Signed-off-by: Peilin Ye > > I'm not entirely sure of the motivation here ... height/width should never > ever be even close to the limit here. Or have you seen integer math that > could potentially go wrong if we go with unsigned instead of int? Oh... No, I have not. I just thought we shouldn't represent a length using a signed value. Also, width and height in console_font are unsigned int - that shouldn't matter that much though. [3/5] doesn't hunk properly without this patch, I'll send a v2 for [3/5] soon. Peilin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Following up
On Tue, Oct 27, 2020 at 07:36:54PM +0100, Daniel Vetter wrote: > On Tue, Oct 27, 2020 at 5:50 PM Peilin Ye wrote: > > ...you mentioned code search, where & what should we look at, in order > > to confirm it's safe to remove them? > > Way back there was google's code search, which was awesome. Now I just > put the structure name/ioctl #define/number into > google/bing/duckduckgo and see if anything turns up. Plus check how > it's used in fb tools (although I just recently learned that fb-test > pretty much disappeared from the internet, very hard to find the > original). > > If you're unsure, we can merge a patch, then wait about 1 year for any > users to show up with problems. If that's not the case, assume they're > all gone, or it was never used and just implemented because it was > copied from somewhere else, or "just in case". There's lots of dead > uapi around. I see, it will be my next thing to do. Hopefully this will remove a lot of console_font occurrences. Peilin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] fbcon: Avoid hard-coding built-in font charcount
On Tue, Oct 27, 2020 at 08:13:53PM +0100, Daniel Vetter wrote: > On Tue, Oct 27, 2020 at 12:37:29PM -0400, Peilin Ye wrote: > > fbcon_startup() and fbcon_init() are hard-coding the number of characters > > of our built-in fonts as 256. Recently, we included that information in > > our kernel font descriptor `struct font_desc`, so use `font->charcount` > > instead of a hard-coded value. > > > > This patch depends on patch "Fonts: Add charcount field to font_desc". > > > > Signed-off-by: Peilin Ye > > So I think this is correct, but it also doesn't do a hole lot yet. fbcon.c > still has tons of hard-coded 256 all over, and if (p->userfont). > > I think if we instead set vc->vc_font.charcount both in fbcon_init and in > fbcon_do_set_font (probably just replace the userfont parameter with > font_charcount for now), then we could replace these all with > vc->vc_font.charcount. And the code would already improve quite a bit I > think. > > With just this change here I think we have even more inconsistency, since > for built-in fonts vc->vc_font.charcount is now set correctly, but for > userfonts we need to instead look at FNTCHARCNT(vc->vc_font.data). You are right, let's remove FNTCHARCNT() altogether. fbcon_do_set_font() still needs a userfont parameter for refcount handling, I'll just add a charcount parameter to it. Peilin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:drm-next 534/551] drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:2532:35: warning: unused variable 'navi10_i2c_algo'
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next head: 2e3a5bc5feeab1ed21f0105f1440a2ff0aef62f9 commit: 19cc89dcb94b47a015fece85db685f1a2563a422 [534/551] drm/amdgpu/swsmu: drop smu i2c bus on navi1x config: x86_64-randconfig-a004-20201026 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project f2c25c70791de95d2466e09b5b58fc37f6ccd7a4) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git git fetch --no-tags radeon-alex drm-next git checkout 19cc89dcb94b47a015fece85db685f1a2563a422 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): >> drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:2532:35: warning: >> unused variable 'navi10_i2c_algo' [-Wunused-const-variable] static const struct i2c_algorithm navi10_i2c_algo = { ^ 1 warning generated. vim +/navi10_i2c_algo +2532 drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c 1bc734759f284eb drivers/gpu/drm/amd/powerplay/navi10_ppt.c Alex Deucher 2020-07-19 2530 1bc734759f284eb drivers/gpu/drm/amd/powerplay/navi10_ppt.c Alex Deucher 2020-07-19 2531 1bc734759f284eb drivers/gpu/drm/amd/powerplay/navi10_ppt.c Alex Deucher 2020-07-19 @2532 static const struct i2c_algorithm navi10_i2c_algo = { 1bc734759f284eb drivers/gpu/drm/amd/powerplay/navi10_ppt.c Alex Deucher 2020-07-19 2533.master_xfer = navi10_i2c_xfer, 1bc734759f284eb drivers/gpu/drm/amd/powerplay/navi10_ppt.c Alex Deucher 2020-07-19 2534.functionality = navi10_i2c_func, 1bc734759f284eb drivers/gpu/drm/amd/powerplay/navi10_ppt.c Alex Deucher 2020-07-19 2535 }; 1bc734759f284eb drivers/gpu/drm/amd/powerplay/navi10_ppt.c Alex Deucher 2020-07-19 2536 :: The code at line 2532 was first introduced by commit :: 1bc734759f284eb531dd474c72ce59874649a254 drm/amdgpu/navi1x: add SMU i2c support (v2) :: TO: Alex Deucher :: CC: Alex Deucher --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/ttm: avoid multihop moves in drivers.
Since I know both of you have looked at this, can someone give me an r-b? Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: xlnx: Use dma_request_chan for DMA channel request
Hi Peter, Thanks for the patch. On Fri, Oct 23, 2020 at 02:46:02AM -0700, Peter Ujfalusi wrote: > There is no need to use the of_dma_request_slave_channel() directly as > dma_request_chan() is going to try to get the channel via OF as well. > > Signed-off-by: Peter Ujfalusi So now dma_request_chan() has sysfs / debugfs registrations, and this looks good to me. I'll commit this within next couple days and keep it posted here. Thanks, -hyun > --- > drivers/gpu/drm/xlnx/zynqmp_disp.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c > b/drivers/gpu/drm/xlnx/zynqmp_disp.c > index 98bd48f13fd1..a4405d081aca 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c > @@ -28,7 +28,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -1316,8 +1315,7 @@ static int zynqmp_disp_layer_request_dma(struct > zynqmp_disp *disp, > > snprintf(dma_channel_name, sizeof(dma_channel_name), >"%s%u", dma_names[layer->id], i); > - dma->chan = of_dma_request_slave_channel(disp->dev->of_node, > - dma_channel_name); > + dma->chan = dma_request_chan(disp->dev, dma_channel_name); > if (IS_ERR(dma->chan)) { > dev_err(disp->dev, "failed to request dma channel\n"); > ret = PTR_ERR(dma->chan); > -- > Peter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:drm-next 511/551] drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c:1234:29: warning: no previous prototype for function 'asic_internal_ss_get_ss_table'
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next head: 2e3a5bc5feeab1ed21f0105f1440a2ff0aef62f9 commit: f6638d0e6f93501dae110d66445c159309aa366a [511/551] drm/amd/pm: correct the checks for sclk/mclk SS support config: x86_64-randconfig-a004-20201026 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project f2c25c70791de95d2466e09b5b58fc37f6ccd7a4) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git git fetch --no-tags radeon-alex drm-next git checkout f6638d0e6f93501dae110d66445c159309aa366a # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): >> drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c:1234:29: >> warning: no previous prototype for function 'asic_internal_ss_get_ss_table' >> [-Wmissing-prototypes] ATOM_ASIC_INTERNAL_SS_INFO *asic_internal_ss_get_ss_table(void *device) ^ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c:1234:1: note: declare 'static' if the function is not intended to be used outside of this translation unit ATOM_ASIC_INTERNAL_SS_INFO *asic_internal_ss_get_ss_table(void *device) ^ static 1 warning generated. vim +/asic_internal_ss_get_ss_table +1234 drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c 1230 1231 /** 1232 * Get the asic internal spread spectrum table 1233 */ > 1234 ATOM_ASIC_INTERNAL_SS_INFO *asic_internal_ss_get_ss_table(void *device) 1235 { 1236 ATOM_ASIC_INTERNAL_SS_INFO *table = NULL; 1237 u8 frev, crev; 1238 u16 size; 1239 1240 table = (ATOM_ASIC_INTERNAL_SS_INFO *) 1241 smu_atom_get_data_table(device, 1242 GetIndexIntoMasterTable(DATA, ASIC_InternalSS_Info), 1243 , , ); 1244 1245 return table; 1246 } 1247 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: manual merge of the drm-misc tree with the amdgpu tree
Hi all, Today's linux-next merge of the drm-misc tree got a conflict in: drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c between commit: ff72bc403170 ("drm/amdgpu: Add debugfs entry for printing VM info") from the amdgpu tree and commit: 4671078eb8e3 ("drm/amdgpu: switch over to the new pin interface") from the drm-misc tree. I fixed it up (I used the former version of this file and added the following patch) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. From: Stephen Rothwell Date: Wed, 28 Oct 2020 11:52:31 +1100 Subject: [PATCH] merge fix up for "drm/amdgpu: Add debugfs entry for printing VM info" Signed-off-by: Stephen Rothwell --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index baca32263ec4..06dfe9b1c7e6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -1555,7 +1555,7 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m) seq_printf(m, "\t\t0x%08x: %12lld byte %s", id, size, placement); - pin_count = READ_ONCE(bo->pin_count); + pin_count = READ_ONCE(bo->tbo.pin_count); if (pin_count) seq_printf(m, " pin count %d", pin_count); -- 2.28.0 -- Cheers, Stephen Rothwell pgpwntnmNF_on.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/9] drm/ttm: new TT backend allocation pool v2
On Tue, 27 Oct 2020 at 03:41, Christian König wrote: > > This replaces the spaghetti code in the two existing page pools. > > First of all depending on the allocation size it is between 3 (1GiB) and > 5 (1MiB) times faster than the old implementation. > > It makes better use of buddy pages to allow for larger physical contiguous > allocations which should result in better TLB utilization at least for > amdgpu. > > Instead of a completely braindead approach of filling the pool with one > CPU while another one is trying to shrink it we only give back freed > pages. > > This also results in much less locking contention and a trylock free MM > shrinker callback, so we can guarantee that pages are given back to the > system when needed. > > Downside of this is that it takes longer for many small allocations until > the pool is filled up. We could address this, but I couldn't find an use > case where this actually matters. We also don't bother freeing large > chunks of pages any more since the CPU overhead in that path isn't really > that important. > > The sysfs files are replaced with a single module parameter, allowing > users to override how many pages should be globally pooled in TTM. This > unfortunately breaks the UAPI slightly, but as far as we know nobody ever > depended on this. > > Zeroing memory coming from the pool was handled inconsistently. The > alloc_pages() based pool was zeroing it, the dma_alloc_attr() based one > wasn't. For now the new implementation isn't zeroing pages from the pool > either and only sets the __GFP_ZERO flag when necessary. > > The implementation has only 768 lines of code compared to the over 2600 > of the old one, and also allows for saving quite a bunch of code in the > drivers since we don't need specialized handling there any more based on > kernel config. > > Additional to all of that there was a neat bug with IOMMU, coherent DMA > mappings and huge pages which is now fixed in the new code as well. > > v2: make ttm_pool_apply_caching static as reported by the kernel bot, add > some more checks #86: FILE: drivers/gpu/drm/ttm/ttm_memory.c:457: +ttm_pool_mgr_init(glob->zone_kernel->max_mem/(2*PAGE_SIZE)); ^ -:86: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV) #86: FILE: drivers/gpu/drm/ttm/ttm_memory.c:457: +ttm_pool_mgr_init(glob->zone_kernel->max_mem/(2*PAGE_SIZE)); ^ -:619: CHECK:BRACES: Blank lines aren't necessary before a close brace '}' #619: FILE: drivers/gpu/drm/ttm/ttm_pool.c:516: + +} -:845: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment #845: FILE: include/drm/ttm/ttm_pool.h:55: +spinlock_t lock; Would be good to get those cleaned up, otherwise Reviewed-by: Dave Airlie for the series. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] xf86drm.c: Use integer logarithm.
I've applied this to libdrm master. Thanks, Dave. On Mon, 26 Oct 2020 at 23:27, Paul Gofman wrote: > > log() is affected by FP control word and can provide inaccurate result. > > Fixes Killer Instinct under Wine not being able to find AMD vulkan > device. > > Signed-off-by: Paul Gofman > --- > With the rounding mode the application sets (unsigned int)log2(4) is 1. > The log2_int() implemetation is copied from radeon/radeon_surface.c. > > xf86drm.c | 18 +- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/xf86drm.c b/xf86drm.c > index 50a6f092..dbb7c14b 100644 > --- a/xf86drm.c > +++ b/xf86drm.c > @@ -124,6 +124,22 @@ static drmServerInfoPtr drm_server_info; > static bool drmNodeIsDRM(int maj, int min); > static char *drmGetMinorNameForFD(int fd, int type); > > +static unsigned log2_int(unsigned x) > +{ > +unsigned l; > + > +if (x < 2) { > +return 0; > +} > +for (l = 2; ; l++) { > +if ((unsigned)(1 << l) > x) { > +return l - 1; > +} > +} > +return 0; > +} > + > + > drm_public void drmSetServerInfo(drmServerInfoPtr info) > { > drm_server_info = info; > @@ -4001,7 +4017,7 @@ static void drmFoldDuplicatedDevices(drmDevicePtr > local_devices[], int count) > for (j = i + 1; j < count; j++) { > if (drmDevicesEqual(local_devices[i], local_devices[j])) { > local_devices[i]->available_nodes |= > local_devices[j]->available_nodes; > -node_type = log2(local_devices[j]->available_nodes); > +node_type = log2_int(local_devices[j]->available_nodes); > memcpy(local_devices[i]->nodes[node_type], > local_devices[j]->nodes[node_type], > drmGetMaxNodeName()); > drmFreeDevice(_devices[j]); > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:drm-next 505/551] drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1743:13: warning: variable 'min' is used uninitialized whenever 'if' condition is false
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next head: 2e3a5bc5feeab1ed21f0105f1440a2ff0aef62f9 commit: a90e6fbe47ff6707a57e55aa578e623b10f79b10 [505/551] drm/amd/pm: correct the settings for ro range minimum and maximum config: x86_64-randconfig-a004-20201026 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project f2c25c70791de95d2466e09b5b58fc37f6ccd7a4) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git git fetch --no-tags radeon-alex drm-next git checkout a90e6fbe47ff6707a57e55aa578e623b10f79b10 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): >> drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1743:13: >> warning: variable 'min' is used uninitialized whenever 'if' condition is >> false [-Wsometimes-uninitialized] } else if ((hwmgr->chip_id == CHIP_POLARIS11) || ^ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1749:27: note: uninitialized use occurs here data->ro_range_minimum = min; ^~~ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1743:9: note: remove the 'if' if its condition is always true } else if ((hwmgr->chip_id == CHIP_POLARIS11) || ^ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1738:15: warning: variable 'min' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] } else if (evv_revision == 2) { ^ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1749:27: note: uninitialized use occurs here data->ro_range_minimum = min; ^~~ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1738:11: note: remove the 'if' if its condition is always true } else if (evv_revision == 2) { ^~~ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1706:43: note: initialize the variable 'min' to silence this warning uint32_t asicrev1, evv_revision, max, min; ^ = 0 >> drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1743:13: >> warning: variable 'max' is used uninitialized whenever 'if' condition is >> false [-Wsometimes-uninitialized] } else if ((hwmgr->chip_id == CHIP_POLARIS11) || ^ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1750:27: note: uninitialized use occurs here data->ro_range_maximum = max; ^~~ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1743:9: note: remove the 'if' if its condition is always true } else if ((hwmgr->chip_id == CHIP_POLARIS11) || ^ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1738:15: warning: variable 'max' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] } else if (evv_revision == 2) { ^ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1750:27: note: uninitialized use occurs here data->ro_range_maximum = max; ^~~ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1738:11: note: remove the 'if' if its condition is always true } else if (evv_revision == 2) { ^~~ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:1706:38: note: initialize the variable 'max' to silence this warning uint32_t asicrev1, evv_revision, max, min; ^ = 0 drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:5286:5: warning: no previous prototype for function 'smu7_init_function_pointers' [-Wmissing-prototypes] int smu7_init_function_pointers(struct pp_hwmgr *hwmgr) ^
[PATCH] drm/shme-helpers: Fix dma_buf_mmap forwarding bug
When we forward an mmap to the dma_buf exporter, they get to own everything. Unfortunately drm_gem_mmap_obj() overwrote vma->vm_private_data after the driver callback, wreaking the exporter complete. This was noticed because vb2_common_vm_close blew up on mali gpu with panfrost after commit 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf"). Unfortunately drm_gem_mmap_obj also acquires a surplus reference that we need to drop in shmem helpers, which is a bit of a mislayer situation. Maybe the entire dma_buf_mmap forwarding should be pulled into core gem code. Note that the only two other drivers which forward mmap in their own code (etnaviv and exynos) get this somewhat right by overwriting the gem mmap code. But they seem to still have the leak. This might be a good excuse to move these drivers over to shmem helpers completely. Note to stable team: There's a trivial context conflict with d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks from struct drm_driver"). I'm assuming it's clear where to put the first hunk, otherwise I can send a 5.9 version of this. Cc: Christian König Cc: Sumit Semwal Cc: Lucas Stach Cc: Russell King Cc: Christian Gmeiner Cc: Inki Dae Cc: Joonyoung Shim Cc: Seung-Woo Kim Cc: Kyungmin Park Fixes: 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf") Cc: Boris Brezillon Cc: Thomas Zimmermann Cc: Gerd Hoffmann Cc: Rob Herring Cc: dri-devel@lists.freedesktop.org Cc: linux-me...@vger.kernel.org Cc: linaro-mm-...@lists.linaro.org Cc: # v5.9+ Reported-and-tested-by: piotr.oniszc...@gmail.com Cc: piotr.oniszc...@gmail.com Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_gem.c | 4 ++-- drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 1da67d34e55d..d586068f5509 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1076,6 +1076,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, */ drm_gem_object_get(obj); + vma->vm_private_data = obj; + if (obj->funcs->mmap) { ret = obj->funcs->mmap(obj, vma); if (ret) { @@ -1096,8 +1098,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); } - vma->vm_private_data = obj; - return 0; } EXPORT_SYMBOL(drm_gem_mmap_obj); diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index fb11df7aced5..8233bda4692f 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -598,8 +598,13 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) /* Remove the fake offset */ vma->vm_pgoff -= drm_vma_node_start(>vma_node); - if (obj->import_attach) + if (obj->import_attach) { + /* Drop the reference drm_gem_mmap_obj() acquired.*/ + drm_gem_object_put(obj); + vma->vm_private_data = NULL; + return dma_buf_mmap(obj->dma_buf, vma, 0); + } shmem = to_drm_gem_shmem_obj(obj); -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:amd-staging-drm-next 294/322] drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c:1234:29: warning: no previous prototype for 'asic_internal_ss_get_ss_table'
tree: git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next head: d8ac65b954234b1616864dc1cbdb7879be0ddc84 commit: 65030ffadab76b8309dca5bd751ca59b564b9fb5 [294/322] drm/amd/pm: correct the checks for sclk/mclk SS support config: alpha-allyesconfig (attached as .config) compiler: alpha-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git git fetch --no-tags radeon-alex amd-staging-drm-next git checkout 65030ffadab76b8309dca5bd751ca59b564b9fb5 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c:31: drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppevvmath.h: In function 'fMultiply': drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppevvmath.h:336:22: warning: variable 'Y_LessThanOne' set but not used [-Wunused-but-set-variable] 336 | bool X_LessThanOne, Y_LessThanOne; | ^ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppevvmath.h:336:7: warning: variable 'X_LessThanOne' set but not used [-Wunused-but-set-variable] 336 | bool X_LessThanOne, Y_LessThanOne; | ^ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c: In function 'atomctrl_calculate_voltage_evv_on_sclk': drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c:702:22: warning: variable 'fPowerDPMx' set but not used [-Wunused-but-set-variable] 702 | fInt fRLL_LoadLine, fPowerDPMx, fDerateTDP, fVDDC_base, fA_Term, fC_Term, fB_Term, fRO_DC_margin; | ^~ drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c: At top level: >> drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c:1234:29: >> warning: no previous prototype for 'asic_internal_ss_get_ss_table' >> [-Wmissing-prototypes] 1234 | ATOM_ASIC_INTERNAL_SS_INFO *asic_internal_ss_get_ss_table(void *device) | ^ In file included from drivers/gpu/drm/amd/amdgpu/../pm/inc/amd_powerplay.h:33, from drivers/gpu/drm/amd/amdgpu/../pm/inc/hwmgr.h:27, from drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.h:27, from drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c:28: drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:198:19: warning: 'no_system_mem_limit' defined but not used [-Wunused-const-variable=] 198 | static const bool no_system_mem_limit; | ^~~ drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:197:19: warning: 'debug_evictions' defined but not used [-Wunused-const-variable=] 197 | static const bool debug_evictions; /* = false */ | ^~~ drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:196:18: warning: 'sched_policy' defined but not used [-Wunused-const-variable=] 196 | static const int sched_policy = KFD_SCHED_POLICY_HWS; | ^~~~ In file included from drivers/gpu/drm/amd/amdgpu/../display/dc/dc_types.h:33, from drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:30, from drivers/gpu/drm/amd/amdgpu/../include/dm_pp_interface.h:26, from drivers/gpu/drm/amd/amdgpu/../pm/inc/amd_powerplay.h:31, from drivers/gpu/drm/amd/amdgpu/../pm/inc/hwmgr.h:27, from drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.h:27, from drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/ppatomctrl.c:28: drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:76:32: warning: 'dc_fixpt_ln2_div_2' defined but not used [-Wunused-const-variable=] 76 | static const struct fixed31_32 dc_fixpt_ln2_div_2 = { 1488522236LL }; |^~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:75:32: warning: 'dc_fixpt_ln2' defined but not used [-Wunused-const-variable=] 75 | static const struct fixed31_32 dc_fixpt_ln2 = { 2977044471LL }; |^~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:74:32: warning: 'dc_fixpt_e' defined but not used [-Wunused-const-variable=] 74 | static const struct fixed31_32 dc_fixpt_e = { 11674931555LL }; |^~
RE: [PATCH v6 4/4] RDMA/mlx5: Support dma-buf based userspace memory region
> -Original Message- > From: Jason Gunthorpe > Sent: Tuesday, October 27, 2020 1:08 PM > To: Xiong, Jianxin > Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford > ; Leon Romanovsky > ; Sumit Semwal ; Christian Koenig > ; Vetter, Daniel > > Subject: Re: [PATCH v6 4/4] RDMA/mlx5: Support dma-buf based userspace memory > region > > On Fri, Oct 23, 2020 at 09:40:01AM -0700, Jianxin Xiong wrote: > > > diff --git a/drivers/infiniband/hw/mlx5/mr.c > > b/drivers/infiniband/hw/mlx5/mr.c index b261797..3bc412b 100644 > > +++ b/drivers/infiniband/hw/mlx5/mr.c > > @@ -1,5 +1,6 @@ > > /* > > * Copyright (c) 2013-2015, Mellanox Technologies. All rights reserved. > > + * Copyright (c) 2020, Intel Corporation. All rights reserved. > > * > > * This software is available to you under a choice of one of two > > * licenses. You may choose to be licensed under the terms of the > > GNU @@ -36,6 +37,8 @@ #include #include > > #include > > +#include > > +#include > > #include > > #include > > #include > > @@ -1113,6 +1116,8 @@ int mlx5_ib_update_xlt(struct mlx5_ib_mr *mr, u64 > > idx, int npages, > > dma_sync_single_for_cpu(ddev, dma, size, DMA_TO_DEVICE); > > if (mr->umem->is_odp) { > > mlx5_odp_populate_xlt(xlt, idx, npages, mr, flags); > > + } else if (mr->umem->is_dmabuf && (flags & > > MLX5_IB_UPD_XLT_ZAP)) { > > + memset(xlt, 0, size); > > } else { > > __mlx5_ib_populate_pas(dev, mr->umem, page_shift, idx, > >npages, xlt, > > @@ -1462,6 +1467,111 @@ struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, > > u64 start, u64 length, > > return ERR_PTR(err); > > } > > > > +static void mlx5_ib_dmabuf_invalidate_cb(struct dma_buf_attachment > > +*attach) { > > + struct ib_umem_dmabuf *umem_dmabuf = attach->importer_priv; > > + struct mlx5_ib_mr *mr = umem_dmabuf->device_context; > > + > > + mlx5_ib_update_xlt(mr, 0, mr->npages, PAGE_SHIFT, MLX5_IB_UPD_XLT_ZAP); > > + ib_umem_dmabuf_unmap_pages(umem_dmabuf); > > +} > > + > > +static struct dma_buf_attach_ops mlx5_ib_dmabuf_attach_ops = { > > + .allow_peer2peer = 1, > > + .move_notify = mlx5_ib_dmabuf_invalidate_cb, }; > > + > > +struct ib_mr *mlx5_ib_reg_user_mr_dmabuf(struct ib_pd *pd, u64 offset, > > +u64 length, u64 virt_addr, > > +int fd, int access_flags, > > +struct ib_udata *udata) > > +{ > > + struct mlx5_ib_dev *dev = to_mdev(pd->device); > > + struct mlx5_ib_mr *mr = NULL; > > + struct ib_umem *umem; > > + struct ib_umem_dmabuf *umem_dmabuf; > > + int npages; > > + int order; > > + int err; > > + > > + if (!IS_ENABLED(CONFIG_INFINIBAND_USER_MEM)) > > + return ERR_PTR(-EOPNOTSUPP); > > + > > + mlx5_ib_dbg(dev, > > + "offset 0x%llx, virt_addr 0x%llx, length 0x%llx, fd %d, > > access_flags 0x%x\n", > > + offset, virt_addr, length, fd, access_flags); > > + > > + if (!mlx5_ib_can_load_pas_with_umr(dev, length)) > > + return ERR_PTR(-EINVAL); > > + > > + umem = ib_umem_dmabuf_get(>ib_dev, offset, length, fd, > > access_flags, > > + _ib_dmabuf_attach_ops); > > + if (IS_ERR(umem)) { > > + mlx5_ib_dbg(dev, "umem get failed (%ld)\n", PTR_ERR(umem)); > > + return ERR_PTR(PTR_ERR(umem)); > > + } > > + > > + npages = ib_umem_num_pages(umem); > > + if (!npages) { > > ib_umem_get should reject invalid umems like this Will move the check to ib_umem_dmabuf_get(). > > > + mlx5_ib_warn(dev, "avoid zero region\n"); > > + ib_umem_release(umem); > > + return ERR_PTR(-EINVAL); > > + } > > + > > + order = ilog2(roundup_pow_of_two(npages)); > > Must always call ib_umem_find_best_pgsz(), specify PAGE_SIZE as the argument > for this scenario Will fix. > > > + mlx5_ib_dbg(dev, "npages %d, ncont %d, order %d, page_shift %d\n", > > + npages, npages, order, PAGE_SHIFT); > > + > > + mr = alloc_mr_from_cache(pd, umem, virt_addr, length, npages, > > +PAGE_SHIFT, order, access_flags); > > + if (IS_ERR(mr)) > > + mr = NULL; > > + > > + if (!mr) { > > + mutex_lock(>slow_path_mutex); > > + mr = reg_create(NULL, pd, virt_addr, length, umem, npages, > > + PAGE_SHIFT, access_flags, false); > > + mutex_unlock(>slow_path_mutex); > > + } > > Rebase on the mlx5 series just posted and use their version of this code > sequence, this is just not quite right Will do. > > > > + err = mlx5_ib_update_xlt(mr, 0, mr->npages, PAGE_SHIFT, > > +MLX5_IB_UPD_XLT_ENABLE | MLX5_IB_UPD_XLT_ZAP); > > + > > + if (err) { > > + dereg_mr(dev, mr); > > + return
RE: [PATCH v6 1/4] RDMA/umem: Support importing dma-buf as user memory region
> -Original Message- > From: Jason Gunthorpe > Sent: Tuesday, October 27, 2020 1:00 PM > To: Xiong, Jianxin > Cc: linux-r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford > ; Leon Romanovsky > ; Sumit Semwal ; Christian Koenig > ; Vetter, Daniel > > Subject: Re: [PATCH v6 1/4] RDMA/umem: Support importing dma-buf as user > memory region > > On Fri, Oct 23, 2020 at 09:39:58AM -0700, Jianxin Xiong wrote: > > +/* > > + * Generate a new dma sg list from a sub range of an existing dma sg list. > > + * Both the input and output have their entries page aligned. > > + */ > > +static int ib_umem_dmabuf_sgt_slice(struct sg_table *sgt, u64 offset, > > + u64 length, struct sg_table *new_sgt) { > > + struct scatterlist *sg, *new_sg; > > + u64 start, end, off, addr, len; > > + unsigned int new_nents; > > + int err; > > + int i; > > + > > + start = ALIGN_DOWN(offset, PAGE_SIZE); > > + end = ALIGN(offset + length, PAGE_SIZE); > > + > > + offset = start; > > + length = end - start; > > + new_nents = 0; > > + for_each_sgtable_dma_sg(sgt, sg, i) { > > + len = sg_dma_len(sg); > > + off = min(len, offset); > > + len -= off; > > + len = min(len, length); > > + if (len) > > + new_nents++; > > + length -= len; > > + offset -= off; > > + } > > + > > + err = sg_alloc_table(new_sgt, new_nents, GFP_KERNEL); > > + if (err) > > + return err; > > I would really rather not allocate an entirely new table just to take a slice > of an existing SGT. Ideally the expoter API from DMA buf would > prepare the SGL slice properly instead of always giving a whole buffer. > > Alternatively making some small edit to rdma_umem_for_each_dma_block() and > ib_umem_find_best_pgsz() would let it slice the SGL at > runtime > > You need to rebase on top of this series: > > https://patchwork.kernel.org/project/linux-rdma/list/?series=370437 > > Which makes mlx5 use those new APIs > > Jason Thanks. Will rebase and work on the runtime slicing. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: remove unneeded semicolon
From: Tom Rix A semicolon is not needed after a switch statement. Signed-off-by: Tom Rix --- drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 2 +- drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c index 7b4b2304bbff..5feb804af4be 100644 --- a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c @@ -858,7 +858,7 @@ static struct clock_source *find_matching_pll( return pool->clock_sources[DCE112_CLK_SRC_PLL5]; default: return NULL; - }; + } return 0; } diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c index fb6a19d020f9..ee5230ccf3c4 100644 --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c @@ -280,6 +280,6 @@ char *mod_hdcp_state_id_to_str(int32_t id) return "D2_A9_VALIDATE_STREAM_READY"; default: return "UNKNOWN_STATE_ID"; - }; + } } -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: panel: simple: Allow timing constraints, not fixed delays
On Tue, Oct 27, 2020 at 08:23:18PM +0100, Daniel Vetter wrote: > On Tue, Oct 27, 2020 at 06:14:59PM +0100, Thierry Reding wrote: > > On Tue, Oct 27, 2020 at 09:45:54AM -0700, Douglas Anderson wrote: > > > The simple panel code currently allows panels to define fixed delays > > > at certain stages of initialization. These work OK, but they don't > > > really map all that clearly to the requirements presented in many > > > panel datasheets. Instead of defining a fixed delay, those datasheets > > > provide a timing diagram and specify a minimum amount of time that > > > needs to pass from event A to event B. > > > > > > Because of the way things are currently defined, most panels end up > > > over-delaying. One prime example here is that a number of panels I've > > > looked at define the amount of time that must pass between turning a > > > panel off and turning it back on again. Since there is no way to > > > specify this, many developers have listed this as the "unprepare" > > > delay. However, if nobody ever tried to turn the panel on again in > > > the next 500 ms (or whatever the delay was) then this delay was > > > pointless. It's better to do the delay only in the case that someone > > > tried to turn the panel on too quickly. > > > > > > Let's support specifying delays as constraints. We'll start with the > > > one above and also a second one: the minimum time between prepare > > > being done and doing the enable. On the panel I'm looking at, there's > > > an 80 ms minimum time between HPD being asserted by the panel and > > > setting the backlight enable GPIO. By specifying as a constraint we > > > can enforce this without over-delaying. Specifically the link > > > training is allowed to happen in parallel with this delay so adding a > > > fixed 80 ms delay isn't ideal. > > > > > > Signed-off-by: Douglas Anderson > > > --- > > > > > > drivers/gpu/drm/panel/panel-simple.c | 51 > > > 1 file changed, 44 insertions(+), 7 deletions(-) > > > > This has always been bugging me a bit about the current setup, so I very > > much like this idea. > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > b/drivers/gpu/drm/panel/panel-simple.c > > > index 2be358fb46f7..cbbe71a2a940 100644 > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > @@ -92,6 +92,19 @@ struct panel_desc { > > > unsigned int unprepare; > > > } delay; > > > > > > + /** > > > + * @prepare_to_enable_ms: If this many milliseconds hasn't passed after > > > + *prepare finished, add a delay to the start > > > + *of enable. > > > + * @unprepare_to_prepare_ms: If this many milliseconds hasn't passed > > > + * unprepare finished, add a delay to the > > > + * start of prepare. > > > > I find this very difficult to understand and it's also not clear from > > this what exactly the delay is. Perhaps this can be somewhat clarified > > Something like the below perhaps? > > > > @prepare_to_enable_ms: The minimum time, in milliseconds, that > > needs to have passed between when prepare finished and enable > > may begin. If at enable time less time has passed since > > prepare finished, the driver waits for the remaining time. > > Also maybe split the kerneldoc into the sub-structure (this should work I > think), so that you can go really wild on formatting :-) I have a patch somewhere where I inlined all the comments and polished them a bit. Will try to dig it up in the weekend. It was motivated by a small W=1 detour. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 13/52] dt-bindings: memory: tegra124: emc: Document new interconnect property
On Tue, Oct 27, 2020 at 10:19:28PM +0300, Dmitry Osipenko wrote: > 27.10.2020 13:25, Krzysztof Kozlowski пишет: > > On Mon, Oct 26, 2020 at 01:16:56AM +0300, Dmitry Osipenko wrote: > >> External memory controller is interconnected with memory controller and > >> with external memory. Document new interconnect property which turns > >> External Memory Controller into interconnect provider. > >> > >> Signed-off-by: Dmitry Osipenko > >> --- > >> .../bindings/memory-controllers/nvidia,tegra124-emc.yaml | 7 +++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml > >> > >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml > >> index 278549f9e051..ac00832ceac1 100644 > >> --- > >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml > >> +++ > >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-emc.yaml > >> @@ -29,6 +29,9 @@ properties: > >> items: > >>- const: emc > >> > >> + "#interconnect-cells": > >> +const: 0 > >> + > >>nvidia,memory-controller: > >> $ref: /schemas/types.yaml#/definitions/phandle > >> description: > >> @@ -327,6 +330,7 @@ required: > >>- clocks > >>- clock-names > >>- nvidia,memory-controller > >> + - "#interconnect-cells" > > > > Another required property, what about all existing users of this binding? > > EMC/devfreq drivers check presence of the new properties and ask users > to upgrade the DT. The kernel will continue to work fine using older > DTBs, but devfreq driver won't load. If the devfreq was working fine before (with these older DTBs and older kernel) then you break the feature. If devfreq was not working or was not stable enough, then nothing is broken so such change is accepted. Which one is then? > > >> additionalProperties: false > >> > >> @@ -345,6 +349,7 @@ examples: > >> > >> #iommu-cells = <1>; > >> #reset-cells = <1>; > >> +#interconnect-cells = <1>; > > > > You meant '0'? > > '1' is for the "mc" node in the example (not "emc" node). > > Anyways, I'll move this hunk to the previous patch in order to fix the > kernel bot warning. Right, thanks. Best regards, Krzysztof ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 08/52] dt-bindings: memory: tegra20: emc: Document mfd-simple compatible and statistics sub-device
On Tue, Oct 27, 2020 at 10:22:19PM +0300, Dmitry Osipenko wrote: > 27.10.2020 12:02, Krzysztof Kozlowski пишет: > >> @@ -31,17 +32,34 @@ Example: > >>... > >>}; > >> > >> + emc_bw_dfs_opp_table: emc_opp_table1 { > > Hyphens for node name. > > We already use underscores for the Tegra CPU OPP table. > > https://elixir.bootlin.com/linux/v5.10-rc1/source/arch/arm/boot/dts/tegra20-cpu-opp.dtsi#L4 > > What makes you think that hyphens will be a better choice? Is it a > documented naming convention? Unfortunately that's the source of confusion also for me because Devicetree spec mentions both of them (and does not specify preferences). The choice of dashes/hyphens comes now explicitly from all dtschema files. Previously, the documentation were emails from Rob. :) Best regards, Krzysztof ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 09/52] dt-bindings: memory: tegra30: mc: Document new interconnect property
On Tue, Oct 27, 2020 at 10:18:35PM +0300, Dmitry Osipenko wrote: > 27.10.2020 12:05, Krzysztof Kozlowski пишет: > > On Mon, Oct 26, 2020 at 01:16:52AM +0300, Dmitry Osipenko wrote: > >> Memory controller is interconnected with memory clients and with the > >> External Memory Controller. Document new interconnect property which > >> turns memory controller into interconnect provider. > >> > >> Acked-by: Rob Herring > >> Signed-off-by: Dmitry Osipenko > >> --- > >> .../bindings/memory-controllers/nvidia,tegra30-mc.yaml | 5 + > >> 1 file changed, 5 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml > >> > >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml > >> index 84fd57bcf0dc..5436e6d420bc 100644 > >> --- > >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml > >> +++ > >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml > >> @@ -57,6 +57,9 @@ properties: > >>"#iommu-cells": > >> const: 1 > >> > >> + "#interconnect-cells": > >> +const: 1 > >> + > >> patternProperties: > >>"^emc-timings-[0-9]+$": > >> type: object > >> @@ -120,6 +123,7 @@ required: > >>- clock-names > >>- "#reset-cells" > >>- "#iommu-cells" > >> + - "#interconnect-cells" > > > > Rob, > > > > You were fine with adding a new required property which breaks all > > existing DTBs? > > This is a required property for the new bindings and optional for the > older. Does it really need to be made optional in the binding? Mhmm... that's an interesting point. I assumed that the bindings should reflect current status of the ABI, but I could imagine that you update the bindings while keeping the driver working with older DTBs. How do you actually track then the ABI? If incompatible change can be added to the bindings, later anyone anytime can also update the driver to enforce the bindings. To require such property. Best regards, Krzysztof > > > Were these bindings marked as unstable? The patchset does not even > > say/scream that it breaks the ABI, so this might be quite a surprise for > > someone... > > Please see tegra_mc_interconnect_setup() in "memory: tegra-mc: Add > interconnect framework" patch, which check presence of the new ICC DT > property. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/tilcdc: avoid 'make W=2' build failure
On 26/10/2020 21:41, Arnd Bergmann wrote: > From: Arnd Bergmann > > The -Wmissing-field-initializer warning when building with W=2 > turns into an error because tilcdc is built with -Werror: > > drm/tilcdc/tilcdc_drv.c:431:33: error: missing field 'data' initializer > [-Werror,-Wmissing-field-initializers] { "regs", tilcdc_regs_show, 0 }, > drm/tilcdc/tilcdc_drv.c:432:33: error: missing field 'data' initializer > [-Werror,-Wmissing-field-initializers] { "mm", tilcdc_mm_show, 0 }, > > Add the missing field initializers to address the warning. > > Signed-off-by: Arnd Bergmann Reviewed-by: Jyri Sarha Please let me know if you want me to merge this. Best regards, Jyri > --- > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > index 4f5fc3e87383..754a66051a21 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > @@ -428,8 +428,8 @@ static int tilcdc_mm_show(struct seq_file *m, void *arg) > } > > static struct drm_info_list tilcdc_debugfs_list[] = { > - { "regs", tilcdc_regs_show, 0 }, > - { "mm", tilcdc_mm_show, 0 }, > + { "regs", tilcdc_regs_show, 0, NULL }, > + { "mm", tilcdc_mm_show, 0, NULL }, > }; > > static void tilcdc_debugfs_init(struct drm_minor *minor) > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 05/52] dt-bindings: memory: tegra20: mc: Document new interconnect property
On Tue, Oct 27, 2020 at 10:17:48PM +0300, Dmitry Osipenko wrote: > 27.10.2020 11:55, Krzysztof Kozlowski пишет: > > On Mon, Oct 26, 2020 at 01:16:48AM +0300, Dmitry Osipenko wrote: > >> Memory controller is interconnected with memory clients and with the > >> External Memory Controller. Document new interconnect property which > >> turns memory controller into interconnect provider. > >> > >> Acked-by: Rob Herring > >> Signed-off-by: Dmitry Osipenko > >> --- > >> .../bindings/memory-controllers/nvidia,tegra20-mc.txt | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt > >> > >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt > >> index e55328237df4..739b7c6f2e26 100644 > >> --- > >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt > >> +++ > >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt > >> @@ -16,6 +16,8 @@ Required properties: > >>IOMMU specifier needed to encode an address. GART supports only a single > >>address space that is shared by all devices, therefore no additional > >>information needed for the address encoding. > >> +- #interconnect-cells : Should be 1. This cell represents memory client. > >> + The assignments may be found in header file > >> . > > > > This is a list of required properties so you break the ABI. All existing > > DTBs will be affected. > > This is optional property for the older DTBs, but for newer DTs it's > mandatory. We do not consider here "older" or "newer" DTBs, but existing ones in the world using this binding. If it was optional so far, it cannot be made mandatory without changing the ABI. Which is an ABI break. > IIUC, it should be defined as a required property in the > binding. > > Please see tegra_mc_interconnect_setup() in "memory: tegra-mc: Add > interconnect framework", which check presence of the ICC DT property. The implementation indeed does not enforce it (except adding error msg, about which I commented). Therefore it should be an optional property. Best regards, Krzysztof ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 04/52] dt-bindings: memory: tegra20: emc: Document nvidia,memory-controller property
On Tue, Oct 27, 2020 at 10:17:19PM +0300, Dmitry Osipenko wrote: > 27.10.2020 11:54, Krzysztof Kozlowski пишет: > > On Mon, Oct 26, 2020 at 01:16:47AM +0300, Dmitry Osipenko wrote: > >> Tegra20 External Memory Controller talks to DRAM chips and it needs to be > >> reprogrammed when memory frequency changes. Tegra Memory Controller sits > >> behind EMC and these controllers are tightly coupled. This patch adds the > >> new phandle property which allows to properly express connection of EMC > >> and MC hardware in a device-tree, it also put the Tegra20 EMC binding on > >> par with Tegra30+ EMC bindings, which is handy to have. > >> > >> Signed-off-by: Dmitry Osipenko > >> --- > >> .../bindings/memory-controllers/nvidia,tegra20-emc.txt | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > >> > >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > >> index 567cffd37f3f..1b0d4417aad8 100644 > >> --- > >> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > >> +++ > >> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt > >> @@ -12,6 +12,7 @@ Properties: > >>irrespective of ram-code configuration. > >> - interrupts : Should contain EMC General interrupt. > >> - clocks : Should contain EMC clock. > >> +- nvidia,memory-controller : Phandle of the Memory Controller node. > > > > It looks like you adding a required property which is an ABI break. > The T20 EMC driver is unused so far in upstream and it will become used > only once this series is applied. Hence it's fine to change the ABI. The ABI is not about upstream, but downstream. There are no other upstreams using this ABI. Unless you have in mind that existing T20 EMC driver was a noop, doing absolutely nothing, therefore there is no breakage of any other users? Best regards, Krzysztof ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: panel: simple: Allow timing constraints, not fixed delays
On Tue, Oct 27, 2020 at 06:14:59PM +0100, Thierry Reding wrote: > On Tue, Oct 27, 2020 at 09:45:54AM -0700, Douglas Anderson wrote: > > The simple panel code currently allows panels to define fixed delays > > at certain stages of initialization. These work OK, but they don't > > really map all that clearly to the requirements presented in many > > panel datasheets. Instead of defining a fixed delay, those datasheets > > provide a timing diagram and specify a minimum amount of time that > > needs to pass from event A to event B. > > > > Because of the way things are currently defined, most panels end up > > over-delaying. One prime example here is that a number of panels I've > > looked at define the amount of time that must pass between turning a > > panel off and turning it back on again. Since there is no way to > > specify this, many developers have listed this as the "unprepare" > > delay. However, if nobody ever tried to turn the panel on again in > > the next 500 ms (or whatever the delay was) then this delay was > > pointless. It's better to do the delay only in the case that someone > > tried to turn the panel on too quickly. > > > > Let's support specifying delays as constraints. We'll start with the > > one above and also a second one: the minimum time between prepare > > being done and doing the enable. On the panel I'm looking at, there's > > an 80 ms minimum time between HPD being asserted by the panel and > > setting the backlight enable GPIO. By specifying as a constraint we > > can enforce this without over-delaying. Specifically the link > > training is allowed to happen in parallel with this delay so adding a > > fixed 80 ms delay isn't ideal. > > > > Signed-off-by: Douglas Anderson > > --- > > > > drivers/gpu/drm/panel/panel-simple.c | 51 > > 1 file changed, 44 insertions(+), 7 deletions(-) > > This has always been bugging me a bit about the current setup, so I very > much like this idea. > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > b/drivers/gpu/drm/panel/panel-simple.c > > index 2be358fb46f7..cbbe71a2a940 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -92,6 +92,19 @@ struct panel_desc { > > unsigned int unprepare; > > } delay; > > > > + /** > > +* @prepare_to_enable_ms: If this many milliseconds hasn't passed after > > +*prepare finished, add a delay to the start > > +*of enable. > > +* @unprepare_to_prepare_ms: If this many milliseconds hasn't passed > > +* unprepare finished, add a delay to the > > +* start of prepare. > > I find this very difficult to understand and it's also not clear from > this what exactly the delay is. Perhaps this can be somewhat clarified > Something like the below perhaps? > > @prepare_to_enable_ms: The minimum time, in milliseconds, that > needs to have passed between when prepare finished and enable > may begin. If at enable time less time has passed since > prepare finished, the driver waits for the remaining time. Also maybe split the kerneldoc into the sub-structure (this should work I think), so that you can go really wild on formatting :-) You could even include diagrams or at least ascii art and stuff ... -Daniel > > > +*/ > > + struct { > > + unsigned int prepare_to_enable_ms; > > + unsigned int unprepare_to_prepare_ms; > > + } timing_constraints; > > + > > u32 bus_format; > > u32 bus_flags; > > int connector_type; > > @@ -99,10 +112,12 @@ struct panel_desc { > > > > struct panel_simple { > > struct drm_panel base; > > - bool prepared; > > I understand how you're trying to reuse the value of prepared_time to > replace this flag, but I find the logic very hard to understand now. > > > bool enabled; > > bool no_hpd; > > > > + ktime_t prepared_time; > > + ktime_t unprepared_time; > > + > > const struct panel_desc *desc; > > > > struct regulator *supply; > > @@ -230,6 +245,21 @@ static int panel_simple_get_non_edid_modes(struct > > panel_simple *panel, > > return num; > > } > > > > +static void panel_simple_enforce_constraint(ktime_t start_ktime, > > + unsigned int min_ms) > > +{ > > + ktime_t now_ktime, min_ktime; > > + > > + if (!min_ms) > > + return; > > + > > + min_ktime = ktime_add(start_ktime, ms_to_ktime(min_ms)); > > + now_ktime = ktime_get(); > > + > > + if (ktime_before(now_ktime, min_ktime)) > > + msleep(ktime_to_ms(ktime_sub(min_ktime, now_ktime)) + 1); > > +} > > + > > static int panel_simple_disable(struct drm_panel *panel) > > { > > struct panel_simple *p = to_panel_simple(panel); > > @@ -249,18 +279,19 @@ static int panel_simple_unprepare(struct drm_panel > > *panel) > >
Re: [PATCH 6/8] drm: atomic: use krealloc_array()
On Tue, Oct 27, 2020 at 01:17:23PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > Use the helper that checks for overflows internally instead of manually > calculating the size of the new array. > > Signed-off-by: Bartosz Golaszewski Acked-by: Daniel Vetter I don't expect conflicts with this going through some other tree, so please make that happen. Or resend once I can apply this to drm trees. Thanks, Daniel > --- > drivers/gpu/drm/drm_atomic.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 58527f151984..09ad6a2ec17b 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -960,7 +960,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state > *state, > struct __drm_connnectors_state *c; > int alloc = max(index + 1, config->num_connector); > > - c = krealloc(state->connectors, alloc * > sizeof(*state->connectors), GFP_KERNEL); > + c = krealloc_array(state->connectors, alloc, > +sizeof(*state->connectors), GFP_KERNEL); > if (!c) > return ERR_PTR(-ENOMEM); > > -- > 2.29.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] parisc/sticore: Avoid hard-coding built-in font charcount
On Tue, Oct 27, 2020 at 12:41:02PM -0400, Peilin Ye wrote: > sti_select_fbfont() and sti_cook_fonts() are hard-coding the number of > characters of our built-in fonts as 256. Recently, we included that > information in our kernel font descriptor `struct font_desc`, so use > `fbfont->charcount` instead of hard-coded values. > > This patch depends on patch "Fonts: Add charcount field to font_desc". > > Signed-off-by: Peilin Ye Reviewed-by: Daniel Vetter > --- > $ # Build-tested (Ubuntu 20.04) > $ sudo apt-get install binutils-hppa64-linux-gnu gcc-7-hppa64-linux-gnu > $ cp arch/parisc/configs/generic-64bit_defconfig .config > $ make -j`nproc` ARCH=parisc CROSS_COMPILE=hppa64-linux-gnu- all > > drivers/video/console/sticore.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c > index d1bb5915082b..f869b723494f 100644 > --- a/drivers/video/console/sticore.c > +++ b/drivers/video/console/sticore.c > @@ -506,7 +506,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, > const char *fbfont_name) > fbfont->width, fbfont->height, fbfont->name); > > bpc = ((fbfont->width+7)/8) * fbfont->height; > - size = bpc * 256; > + size = bpc * fbfont->charcount; > size += sizeof(struct sti_rom_font); > > nf = kzalloc(size, STI_LOWMEM); > @@ -514,7 +514,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, > const char *fbfont_name) > return NULL; > > nf->first_char = 0; > - nf->last_char = 255; > + nf->last_char = fbfont->charcount - 1; > nf->width = fbfont->width; > nf->height = fbfont->height; > nf->font_type = STI_FONT_HPROMAN8; > @@ -525,7 +525,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, > const char *fbfont_name) > > dest = nf; > dest += sizeof(struct sti_rom_font); > - memcpy(dest, fbfont->data, bpc*256); > + memcpy(dest, fbfont->data, bpc * fbfont->charcount); > > cooked_font = kzalloc(sizeof(*cooked_font), GFP_KERNEL); > if (!cooked_font) { > @@ -660,7 +660,7 @@ static int sti_cook_fonts(struct sti_cooked_rom > *cooked_rom, > void sti_font_convert_bytemode(struct sti_struct *sti, struct > sti_cooked_font *f) > { > unsigned char *n, *p, *q; > - int size = f->raw->bytes_per_char * 256 + sizeof(struct sti_rom_font); > + int size = f->raw->bytes_per_char * (f->raw->last_char + 1) + > sizeof(struct sti_rom_font); > struct sti_rom_font *old_font; > > if (sti->wordmode) > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] fbcon: Avoid hard-coding built-in font charcount
On Tue, Oct 27, 2020 at 12:37:29PM -0400, Peilin Ye wrote: > fbcon_startup() and fbcon_init() are hard-coding the number of characters > of our built-in fonts as 256. Recently, we included that information in > our kernel font descriptor `struct font_desc`, so use `font->charcount` > instead of a hard-coded value. > > This patch depends on patch "Fonts: Add charcount field to font_desc". > > Signed-off-by: Peilin Ye So I think this is correct, but it also doesn't do a hole lot yet. fbcon.c still has tons of hard-coded 256 all over, and if (p->userfont). I think if we instead set vc->vc_font.charcount both in fbcon_init and in fbcon_do_set_font (probably just replace the userfont parameter with font_charcount for now), then we could replace these all with vc->vc_font.charcount. And the code would already improve quite a bit I think. With just this change here I think we have even more inconsistency, since for built-in fonts vc->vc_font.charcount is now set correctly, but for userfonts we need to instead look at FNTCHARCNT(vc->vc_font.data). We'd still need to maintain p->userfont because of the refcount chaos, but that is much more work. Or do I miss something here? -Daniel > --- > drivers/video/fbdev/core/fbcon.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbcon.c > b/drivers/video/fbdev/core/fbcon.c > index cef437817b0d..e563847991b7 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -1004,7 +1004,7 @@ static const char *fbcon_startup(void) > vc->vc_font.width = font->width; > vc->vc_font.height = font->height; > vc->vc_font.data = (void *)(p->fontdata = font->data); > - vc->vc_font.charcount = 256; /* FIXME Need to support more > fonts */ > + vc->vc_font.charcount = font->charcount; > } else { > p->fontdata = vc->vc_font.data; > } > @@ -1083,8 +1083,7 @@ static void fbcon_init(struct vc_data *vc, int init) > vc->vc_font.width = font->width; > vc->vc_font.height = font->height; > vc->vc_font.data = (void *)(p->fontdata = font->data); > - vc->vc_font.charcount = 256; /* FIXME Need to > - support more fonts */ > + vc->vc_font.charcount = font->charcount; > } > } > > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: remove unneeded semicolon
From: Tom Rix A semicolon is not needed after a switch statement. Signed-off-by: Tom Rix --- drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c index 1b213c4ddfcb..19c0a3655228 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c @@ -654,7 +654,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) default: return 0; - }; + } return ret; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 8bf6a7c056bc..a61cf8cfbfc3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -953,7 +953,7 @@ static char *amdgpu_ras_badpage_flags_str(unsigned int flags) case AMDGPU_RAS_RETIRE_PAGE_FAULT: default: return "F"; - }; + } } /** -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/65] mm: Track mmu notifiers in fs_reclaim_acquire/release
On Tue, Oct 27, 2020 at 7:51 PM Christoph Hellwig wrote: > Is there a list that has the cover letter and the whole series? > I've only found fragments (and mostly the same fragments) while > wading through my backlog in various list folders.. Typoed git send-email command that I only caught half-way through. I tried to reply with apologies in a few spots, I guess I didn't cover all the lists this spams :-/ The patch itself is still somewhere on my todo to respin, I want to pep it up with some testcases since previous version was kinda badly broken. Just didn't get around to that yet. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] Fonts: Add charcount field to font_desc
On Tue, Oct 27, 2020 at 12:34:26PM -0400, Peilin Ye wrote: > Subsystems are assuming the number of characters of our built-in fonts. > Include that information in our kernel font descriptor, `struct > font_desc`. > > Signed-off-by: Peilin Ye Reviewed-by: Daniel Vetter atm can't merge this because we need a backmerge of maybe -rc2 into drm-misc-next first. Please remind me if this doesn't land next week. -Daniel > --- > include/linux/font.h | 1 + > lib/fonts/font_10x18.c | 1 + > lib/fonts/font_6x10.c | 1 + > lib/fonts/font_6x11.c | 1 + > lib/fonts/font_6x8.c | 1 + > lib/fonts/font_7x14.c | 1 + > lib/fonts/font_8x16.c | 1 + > lib/fonts/font_8x8.c | 1 + > lib/fonts/font_acorn_8x8.c | 1 + > lib/fonts/font_mini_4x6.c | 1 + > lib/fonts/font_pearl_8x8.c | 1 + > lib/fonts/font_sun12x22.c | 1 + > lib/fonts/font_sun8x16.c | 1 + > lib/fonts/font_ter16x32.c | 1 + > 14 files changed, 14 insertions(+) > > diff --git a/include/linux/font.h b/include/linux/font.h > index 4f50d736ea72..abf1442ce719 100644 > --- a/include/linux/font.h > +++ b/include/linux/font.h > @@ -17,6 +17,7 @@ struct font_desc { > int idx; > const char *name; > unsigned int width, height; > +unsigned int charcount; > const void *data; > int pref; > }; > diff --git a/lib/fonts/font_10x18.c b/lib/fonts/font_10x18.c > index 0e2deac97da0..4096c6562494 100644 > --- a/lib/fonts/font_10x18.c > +++ b/lib/fonts/font_10x18.c > @@ -5137,6 +5137,7 @@ const struct font_desc font_10x18 = { > .name = "10x18", > .width = 10, > .height = 18, > + .charcount = 256, > .data = fontdata_10x18.data, > #ifdef __sparc__ > .pref = 5, > diff --git a/lib/fonts/font_6x10.c b/lib/fonts/font_6x10.c > index 87da8acd07db..32786674cf65 100644 > --- a/lib/fonts/font_6x10.c > +++ b/lib/fonts/font_6x10.c > @@ -3083,6 +3083,7 @@ const struct font_desc font_6x10 = { > .name = "6x10", > .width = 6, > .height = 10, > + .charcount = 256, > .data = fontdata_6x10.data, > .pref = 0, > }; > diff --git a/lib/fonts/font_6x11.c b/lib/fonts/font_6x11.c > index 5e975dfa10a5..81e4a3aed44a 100644 > --- a/lib/fonts/font_6x11.c > +++ b/lib/fonts/font_6x11.c > @@ -3346,6 +3346,7 @@ const struct font_desc font_vga_6x11 = { > .name = "ProFont6x11", > .width = 6, > .height = 11, > + .charcount = 256, > .data = fontdata_6x11.data, > /* Try avoiding this font if possible unless on MAC */ > .pref = -2000, > diff --git a/lib/fonts/font_6x8.c b/lib/fonts/font_6x8.c > index 700039a9ceae..5618ae7ef9fa 100644 > --- a/lib/fonts/font_6x8.c > +++ b/lib/fonts/font_6x8.c > @@ -2571,6 +2571,7 @@ const struct font_desc font_6x8 = { > .name = "6x8", > .width = 6, > .height = 8, > + .charcount = 256, > .data = fontdata_6x8.data, > .pref = 0, > }; > diff --git a/lib/fonts/font_7x14.c b/lib/fonts/font_7x14.c > index 86d298f38505..7708e73d491f 100644 > --- a/lib/fonts/font_7x14.c > +++ b/lib/fonts/font_7x14.c > @@ -4113,6 +4113,7 @@ const struct font_desc font_7x14 = { > .name = "7x14", > .width = 7, > .height = 14, > + .charcount = 256, > .data = fontdata_7x14.data, > .pref = 0, > }; > diff --git a/lib/fonts/font_8x16.c b/lib/fonts/font_8x16.c > index 37cedd36ca5e..74125d3570cf 100644 > --- a/lib/fonts/font_8x16.c > +++ b/lib/fonts/font_8x16.c > @@ -4627,6 +4627,7 @@ const struct font_desc font_vga_8x16 = { > .name = "VGA8x16", > .width = 8, > .height = 16, > + .charcount = 256, > .data = fontdata_8x16.data, > .pref = 0, > }; > diff --git a/lib/fonts/font_8x8.c b/lib/fonts/font_8x8.c > index 8ab695538395..96da4bb31ae4 100644 > --- a/lib/fonts/font_8x8.c > +++ b/lib/fonts/font_8x8.c > @@ -2578,6 +2578,7 @@ const struct font_desc font_vga_8x8 = { > .name = "VGA8x8", > .width = 8, > .height = 8, > + .charcount = 256, > .data = fontdata_8x8.data, > .pref = 0, > }; > diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c > index 069b3e80c434..ba74053fec7b 100644 > --- a/lib/fonts/font_acorn_8x8.c > +++ b/lib/fonts/font_acorn_8x8.c > @@ -270,6 +270,7 @@ const struct font_desc font_acorn_8x8 = { > .name = "Acorn8x8", > .width = 8, > .height = 8, > + .charcount = 256, > .data = acorndata_8x8.data, > #ifdef CONFIG_ARCH_ACORN > .pref = 20, > diff --git a/lib/fonts/font_mini_4x6.c b/lib/fonts/font_mini_4x6.c > index 1449876c6a27..637708e8c67e 100644 > --- a/lib/fonts/font_mini_4x6.c > +++ b/lib/fonts/font_mini_4x6.c > @@ -2152,6 +2152,7 @@ const struct font_desc font_mini_4x6 = { > .name = "MINI4x6", > .width = 4, > .height = 6, > + .charcount = 256, > .data = fontdata_mini_4x6.data, > .pref = 3, > }; > diff --git a/lib/fonts/font_pearl_8x8.c
Re: [PATCH 2/5] Fonts: Make font size unsigned in font_desc
On Tue, Oct 27, 2020 at 12:33:05PM -0400, Peilin Ye wrote: > It is improper to define `width` and `height` as signed in `struct > font_desc`. Make them unsigned. Also, change the corresponding printk() > format identifiers from `%d` to `%u`, in sti_select_fbfont(). > > Signed-off-by: Peilin Ye I'm not entirely sure of the motivation here ... height/width should never ever be even close to the limit here. Or have you seen integer math that could potentially go wrong if we go with unsigned instead of int? -Daniel > --- > Build-tested. > > drivers/video/console/sticore.c | 2 +- > include/linux/font.h| 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c > index 6a26a364f9bd..d1bb5915082b 100644 > --- a/drivers/video/console/sticore.c > +++ b/drivers/video/console/sticore.c > @@ -502,7 +502,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, > const char *fbfont_name) > if (!fbfont) > return NULL; > > - pr_info("STI selected %dx%d framebuffer font %s for sticon\n", > + pr_info("STI selected %ux%u framebuffer font %s for sticon\n", > fbfont->width, fbfont->height, fbfont->name); > > bpc = ((fbfont->width+7)/8) * fbfont->height; > diff --git a/include/linux/font.h b/include/linux/font.h > index b5b312c19e46..4f50d736ea72 100644 > --- a/include/linux/font.h > +++ b/include/linux/font.h > @@ -16,7 +16,7 @@ > struct font_desc { > int idx; > const char *name; > -int width, height; > +unsigned int width, height; > const void *data; > int pref; > }; > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH] drm/msm/dpu: fix clock scaling on non-sc7180 board
On 2020-10-27 03:23, Dmitry Baryshkov wrote: c33b7c0389e1 ("drm/msm/dpu: add support for clk and bw scaling for display") has added support for handling bandwidth voting in kms path in addition to old mdss path. However this broke all other platforms since _dpu_core_perf_crtc_update_bus() will now error out instead of properly calculating bandwidth and core clocks. Fix _dpu_core_perf_crtc_update_bus() to just skip bandwidth setting instead of returning an error in case kms->num_paths == 0 (MDSS is used for bandwidth management). Signed-off-by: Dmitry Baryshkov Fixes: c33b7c0389e1 ("drm/msm/dpu: add support for clk and bw scaling for display") --- Looks fine to me, thanks for the fix. Reviewed-by: Abhinav Kumar drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 393858ef8a83..37c8270681c2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -219,9 +219,6 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms, int i, ret = 0; u64 avg_bw; - if (!kms->num_paths) - return -EINVAL; - drm_for_each_crtc(tmp_crtc, crtc->dev) { if (tmp_crtc->enabled && curr_client_type == @@ -239,6 +236,9 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms, } } + if (!kms->num_paths) + return 0; + avg_bw = perf.bw_ctl; do_div(avg_bw, (kms->num_paths * 1000)); /*Bps_to_icc*/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] fbdev/atafb: Remove unused extern variables
On Tue, Oct 27, 2020 at 12:31:08PM -0400, Peilin Ye wrote: > Remove 6 unused extern variables to reduce confusion. It is worth > mentioning that lib/fonts/font_8x8.c and lib/fonts/font_8x16.c also > declare `fontdata_8x8` and `fontdata_8x16` respectively, and this file > has nothing to do with them. > > Signed-off-by: Peilin Ye This was unused ever since this driver was merged into 2.1.67 (I looked at historical linux git trees quickly). Save to delete I'd say, probably just copypasted from some outdated driver template that was even older. Applied to drm-misc-next. -Daniel > --- > $ # Build-tested (Ubuntu 20.04) > $ sudo apt install gcc-m68k-linux-gnu > $ cp arch/m68k/configs/atari_defconfig .config > $ make ARCH=m68k menuconfig > $ make ARCH=m68k CROSS_COMPILE=m68k-linux-gnu- -j`nproc` all > > drivers/video/fbdev/atafb.c | 8 > 1 file changed, 8 deletions(-) > > diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c > index f253daa05d9d..e3812a8ff55a 100644 > --- a/drivers/video/fbdev/atafb.c > +++ b/drivers/video/fbdev/atafb.c > @@ -240,14 +240,6 @@ static int *MV300_reg = MV300_reg_8bit; > > static int inverse; > > -extern int fontheight_8x8; > -extern int fontwidth_8x8; > -extern unsigned char fontdata_8x8[]; > - > -extern int fontheight_8x16; > -extern int fontwidth_8x16; > -extern unsigned char fontdata_8x16[]; > - > /* > * struct fb_ops { > * * open/release and usage marking > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Following up
On Tue, Oct 27, 2020 at 5:50 PM Peilin Ye wrote: > > Hi Daniel, > > More about the 3 things we've discussed before: > > 1. Cleaning up con_font_op(): > > (drivers/tty/vt/vt.c) > int con_font_op(struct vc_data *vc, struct console_font_op *op) > { > switch (op->op) { > case KD_FONT_OP_SET: > return con_font_set(vc, op); > case KD_FONT_OP_GET: > return con_font_get(vc, op); > case KD_FONT_OP_SET_DEFAULT: > return con_font_default(vc, op); > case KD_FONT_OP_COPY: > return con_font_copy(vc, op); > } > return -ENOSYS; > } > > On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote: > > I think if we change the conf_font_get/set/default/copy functions to not > > take the *op struct (which is take pretty arbitrarily from one of the > > ioctl), but the parameters each needs directly, that would clean up the > > code a _lot_. > > This is on my TODO list! One day I came up with some idea about > fbcon.c, so I postponed this a bit... > > 2. Removing dummy functions, like sisusbdummycon_font_set(): > > Turns out, before c396a5bf457f ("console: Expand dummy functions for > CFI"), they were just some macros: > > -#define SISUSBCONDUMMY (void *)sisusbdummycon_dummy > +static int sisusbdummycon_font_set(struct vc_data *vc, > + struct console_font *font, > + unsigned int flags) > +{ > + return 0; > +} > > ...and they had been there for a very long (10+ years) time. Removing > code like this makes me a bit nervous, and... > > On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote: > > This actually does something. tbh I would not be surprises if the > > fb_set utility is the only thing that uses this - with a bit of code > > search we could perhaps confirm this, and delete all the other > > implementations. > > ...you mentioned code search, where & what should we look at, in order > to confirm it's safe to remove them? Way back there was google's code search, which was awesome. Now I just put the structure name/ioctl #define/number into google/bing/duckduckgo and see if anything turns up. Plus check how it's used in fb tools (although I just recently learned that fb-test pretty much disappeared from the internet, very hard to find the original). If you're unsure, we can merge a patch, then wait about 1 year for any users to show up with problems. If that's not the case, assume they're all gone, or it was never used and just implemented because it was copied from somewhere else, or "just in case". There's lots of dead uapi around. > 3. Using `font_desc` in `vc_data`: > > Our plan for the gradual conversion was to use a helper function to > set font for a vc, but after reviewing the 300-ish occurrence of > `vc_font`, it seems like code doesn't usually set it as a whole: > > (drivers/usb/misc/sisusbvga/sisusb_con.c) > [...] > c->vc_font.height = sisusb->current_font_height; > [...] > > ...that's it! It only cares about the height. There are only 4 or 5 > places in fbcon.c that actually set all fields of `vc_font`, like: > > vc->vc_font.width = font->width; > vc->vc_font.height = font->height; > vc->vc_font.data = (void *)(p->fontdata = font->data); > vc->vc_font.charcount = 256; /* FIXME Need to support more > fonts */ > > To make it even more complicated, `p` is a `struct fbcon_display *`, > containing yet another font data pointer (`fontdata`) that I think > should be replaced by a `font_desc *`... > > In conclusion, I think it's all about a few hard problems in fbcon.c. > I'll keep trying and see how it goes. Yeah fbcon.c is pretty good horrors unfortunately :-/ -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/8] edac: ghes: use krealloc_array()
On Tue, Oct 27, 2020 at 01:17:22PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > Use the helper that checks for overflows internally instead of manually > calculating the size of the new array. > > Signed-off-by: Bartosz Golaszewski > --- > drivers/edac/ghes_edac.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c > index a918ca93e4f7..6d1ddecbf0da 100644 > --- a/drivers/edac/ghes_edac.c > +++ b/drivers/edac/ghes_edac.c > @@ -207,8 +207,8 @@ static void enumerate_dimms(const struct dmi_header *dh, > void *arg) > if (!hw->num_dimms || !(hw->num_dimms % 16)) { > struct dimm_info *new; > > - new = krealloc(hw->dimms, (hw->num_dimms + 16) * sizeof(struct > dimm_info), > - GFP_KERNEL); > + new = krealloc_array(hw->dimms, hw->num_dimms + 16, > + sizeof(struct dimm_info), GFP_KERNEL); > if (!new) { > WARN_ON_ONCE(1); > return; > -- Sure, why not. Acked-by: Borislav Petkov -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/8] mm: slab: provide krealloc_array()
On 10/27/20 1:17 PM, Bartosz Golaszewski wrote: From: Bartosz Golaszewski When allocating an array of elements, users should check for multiplication overflow or preferably use one of the provided helpers like: kmalloc_array(). There's no krealloc_array() counterpart but there are many users who use regular krealloc() to reallocate arrays. Let's provide an actual krealloc_array() implementation. Signed-off-by: Bartosz Golaszewski Makes sense. Acked-by: Vlastimil Babka --- include/linux/slab.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/include/linux/slab.h b/include/linux/slab.h index dd6897f62010..0e6683affee7 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -592,6 +592,17 @@ static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags) return __kmalloc(bytes, flags); } +static __must_check inline void * +krealloc_array(void *p, size_t new_n, size_t new_size, gfp_t flags) +{ + size_t bytes; + + if (unlikely(check_mul_overflow(new_n, new_size, ))) + return NULL; + + return krealloc(p, bytes, flags); +} + /** * kcalloc - allocate memory for an array. The memory is set to zero. * @n: number of elements. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v6 1/4] RDMA/umem: Support importing dma-buf as user memory region
> -Original Message- > From: Christoph Hellwig > Sent: Tuesday, October 27, 2020 1:08 AM > To: Jason Gunthorpe > Cc: Christoph Hellwig ; Daniel Vetter ; > Xiong, Jianxin ; linux- > r...@vger.kernel.org; dri-devel@lists.freedesktop.org; Leon Romanovsky > ; Doug Ledford ; > Vetter, Daniel ; Christian Koenig > > Subject: Re: [PATCH v6 1/4] RDMA/umem: Support importing dma-buf as user > memory region > > On Mon, Oct 26, 2020 at 09:26:37AM -0300, Jason Gunthorpe wrote: > > On Sat, Oct 24, 2020 at 08:48:07AM +0100, Christoph Hellwig wrote: > > > On Fri, Oct 23, 2020 at 03:20:05PM -0300, Jason Gunthorpe wrote: > > > > The problem is we have RDMA drivers that assume SGL's have a valid > > > > struct page, and these hacky/wrong P2P sgls that DMABUF creates > > > > cannot be passed into those drivers. > > > > > > RDMA drivers do not assume scatterlist have a valid struct page, > > > scatterlists are defined to have a valid struct page. Any > > > scatterlist without a struct page is completely buggy. > > > > It is not just having the struct page, it needs to be a CPU accessible > > one for memcpy/etc. They aren't correct with the > > MEMORY_DEVICE_PCI_P2PDMA SGLs either. > > Exactly. In the function ib_umem_dmabuf_sgt_slice() (part of this patch) we could generate a dma address array instead of filling the scatterlist 'umem->sg_head'. The array would be handled similar to 'umem_odp->dma_list'. With such change, the RDMA drivers wouldn't see incorrectly formed scatterlist. The check for dma_virt_ops here wouldn't be needed either. Would such proposal address the concern here? -Jianxin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: panel: simple: Allow timing constraints, not fixed delays
On Tue, Oct 27, 2020 at 09:45:54AM -0700, Douglas Anderson wrote: > The simple panel code currently allows panels to define fixed delays > at certain stages of initialization. These work OK, but they don't > really map all that clearly to the requirements presented in many > panel datasheets. Instead of defining a fixed delay, those datasheets > provide a timing diagram and specify a minimum amount of time that > needs to pass from event A to event B. > > Because of the way things are currently defined, most panels end up > over-delaying. One prime example here is that a number of panels I've > looked at define the amount of time that must pass between turning a > panel off and turning it back on again. Since there is no way to > specify this, many developers have listed this as the "unprepare" > delay. However, if nobody ever tried to turn the panel on again in > the next 500 ms (or whatever the delay was) then this delay was > pointless. It's better to do the delay only in the case that someone > tried to turn the panel on too quickly. > > Let's support specifying delays as constraints. We'll start with the > one above and also a second one: the minimum time between prepare > being done and doing the enable. On the panel I'm looking at, there's > an 80 ms minimum time between HPD being asserted by the panel and > setting the backlight enable GPIO. By specifying as a constraint we > can enforce this without over-delaying. Specifically the link > training is allowed to happen in parallel with this delay so adding a > fixed 80 ms delay isn't ideal. > > Signed-off-by: Douglas Anderson > --- > > drivers/gpu/drm/panel/panel-simple.c | 51 > 1 file changed, 44 insertions(+), 7 deletions(-) This has always been bugging me a bit about the current setup, so I very much like this idea. > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > b/drivers/gpu/drm/panel/panel-simple.c > index 2be358fb46f7..cbbe71a2a940 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -92,6 +92,19 @@ struct panel_desc { > unsigned int unprepare; > } delay; > > + /** > + * @prepare_to_enable_ms: If this many milliseconds hasn't passed after > + *prepare finished, add a delay to the start > + *of enable. > + * @unprepare_to_prepare_ms: If this many milliseconds hasn't passed > + * unprepare finished, add a delay to the > + * start of prepare. I find this very difficult to understand and it's also not clear from this what exactly the delay is. Perhaps this can be somewhat clarified Something like the below perhaps? @prepare_to_enable_ms: The minimum time, in milliseconds, that needs to have passed between when prepare finished and enable may begin. If at enable time less time has passed since prepare finished, the driver waits for the remaining time. > + */ > + struct { > + unsigned int prepare_to_enable_ms; > + unsigned int unprepare_to_prepare_ms; > + } timing_constraints; > + > u32 bus_format; > u32 bus_flags; > int connector_type; > @@ -99,10 +112,12 @@ struct panel_desc { > > struct panel_simple { > struct drm_panel base; > - bool prepared; I understand how you're trying to reuse the value of prepared_time to replace this flag, but I find the logic very hard to understand now. > bool enabled; > bool no_hpd; > > + ktime_t prepared_time; > + ktime_t unprepared_time; > + > const struct panel_desc *desc; > > struct regulator *supply; > @@ -230,6 +245,21 @@ static int panel_simple_get_non_edid_modes(struct > panel_simple *panel, > return num; > } > > +static void panel_simple_enforce_constraint(ktime_t start_ktime, > + unsigned int min_ms) > +{ > + ktime_t now_ktime, min_ktime; > + > + if (!min_ms) > + return; > + > + min_ktime = ktime_add(start_ktime, ms_to_ktime(min_ms)); > + now_ktime = ktime_get(); > + > + if (ktime_before(now_ktime, min_ktime)) > + msleep(ktime_to_ms(ktime_sub(min_ktime, now_ktime)) + 1); > +} > + > static int panel_simple_disable(struct drm_panel *panel) > { > struct panel_simple *p = to_panel_simple(panel); > @@ -249,18 +279,19 @@ static int panel_simple_unprepare(struct drm_panel > *panel) > { > struct panel_simple *p = to_panel_simple(panel); > > - if (!p->prepared) > + if (!p->prepared_time) > return 0; Here for example I now need to actively think about what exactly !prepared_time actually means, when all it really means is that we're checking if the panel has already been enabled. Perhaps we could provide a tiny helper to make this clearer? static inline bool
Re: [PATCH 3/8] vhost: vringh: use krealloc_array()
On Tue, 2020-10-27 at 17:58 +0100, Bartosz Golaszewski wrote: > On Tue, Oct 27, 2020 at 5:50 PM Joe Perches wrote: > > > > On Tue, 2020-10-27 at 11:28 -0400, Michael S. Tsirkin wrote: > > > On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski > > > > > > > > Use the helper that checks for overflows internally instead of manually > > > > calculating the size of the new array. > > > > > > > > Signed-off-by: Bartosz Golaszewski > > > > > > No problem with the patch, it does introduce some symmetry in the code. > > > > Perhaps more symmetry by using kmemdup > > --- > > drivers/vhost/vringh.c | 23 ++- > > 1 file changed, 10 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c > > index 8bd8b403f087..99222a3651cd 100644 > > --- a/drivers/vhost/vringh.c > > +++ b/drivers/vhost/vringh.c > > @@ -191,26 +191,23 @@ static int move_to_indirect(const struct vringh *vrh, > > static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp) > > { > > struct kvec *new; > > - unsigned int flag, new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) > > * 2; > > + size_t new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2; > > + size_t size; > > > > if (new_num < 8) > > new_num = 8; > > > > - flag = (iov->max_num & VRINGH_IOV_ALLOCATED); > > - if (flag) > > - new = krealloc(iov->iov, new_num * sizeof(struct iovec), > > gfp); > > - else { > > - new = kmalloc_array(new_num, sizeof(struct iovec), gfp); > > - if (new) { > > - memcpy(new, iov->iov, > > - iov->max_num * sizeof(struct iovec)); > > - flag = VRINGH_IOV_ALLOCATED; > > - } > > - } > > + if (unlikely(check_mul_overflow(new_num, sizeof(struct iovec), > > ))) > > + return -ENOMEM; > > + > > The whole point of using helpers such as kmalloc_array() is not doing > these checks manually. Tradeoffs for in readability for overflow and not mistyping or doing the multiplication of iov->max_num * sizeof(struct iovec) twice. Just fyi: the realloc doesn't do a multiplication overflow test as written so the suggestion is slightly more resistant to defect. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 12/16] drm/i915/hdcp: MST streams support in hdcp port_data
Add support for multiple mst stream in hdcp port data which will be used by RepeaterAuthStreamManage msg and HDCP 2.2 security f/w for m' validation. v2: Init the hdcp port data k for HDMI/DP SST strem. v3: Cosmetic changes. [Uma] Cc: Ramalingam C Signed-off-by: Anshuman Gupta --- .../drm/i915/display/intel_display_types.h| 4 +- drivers/gpu/drm/i915/display/intel_hdcp.c | 103 +++--- 2 files changed, 92 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 749c3a7e0b45..24e0067c2e7c 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1445,10 +1445,12 @@ struct intel_digital_port { enum phy_fia tc_phy_fia; u8 tc_phy_fia_idx; - /* protects num_hdcp_streams reference count, port_data */ + /* protects num_hdcp_streams reference count, port_data and port_auth */ struct mutex hdcp_mutex; /* the number of pipes using HDCP signalling out of this port */ unsigned int num_hdcp_streams; + /* port HDCP auth status */ + bool port_auth; /* HDCP port data need to pass to security f/w */ struct hdcp_port_data port_data; diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index a5ec4f72f50f..1df6d4a23476 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -26,6 +26,64 @@ #define KEY_LOAD_TRIES 5 #define HDCP2_LC_RETRY_CNT 3 +static int intel_conn_to_vcpi(struct intel_connector *connector) +{ + /* For HDMI this is forced to be 0x0. For DP SST also this is 0x0. */ + return connector->port ? connector->port->vcpi.vcpi : 0; +} + +static int +intel_hdcp_required_content_stream(struct intel_digital_port *dig_port) +{ + struct drm_connector_list_iter conn_iter; + struct intel_digital_port *conn_dig_port; + struct intel_connector *connector; + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); + struct hdcp_port_data *data = _port->port_data; + bool enforce_type0 = false; + int k; + + if (dig_port->port_auth) + return 0; + + drm_connector_list_iter_begin(>drm, _iter); + for_each_intel_connector_iter(connector, _iter) { + if (!intel_encoder_is_mst(intel_attached_encoder(connector))) + continue; + + conn_dig_port = intel_attached_dig_port(connector); + if (conn_dig_port != dig_port) + continue; + + if (connector->base.status == connector_status_disconnected) + continue; + + if (!enforce_type0 && !intel_hdcp2_capable(connector)) + enforce_type0 = true; + + data->streams[data->k].stream_id = intel_conn_to_vcpi(connector); + data->k++; + + /* if there is only one active stream */ + if (dig_port->dp.active_mst_links <= 1) + break; + } + drm_connector_list_iter_end(_iter); + + if (drm_WARN_ON(>drm, data->k > INTEL_NUM_PIPES(i915) || data->k == 0)) + return -EINVAL; + + /* +* Apply common protection level across all streams in DP MST Topology. +* Use highest supported content type for all streams in DP MST Topology. +*/ + for (k = 0; k < data->k; k++) + data->streams[k].stream_type = + enforce_type0 ? DRM_MODE_HDCP_CONTENT_TYPE0 : DRM_MODE_HDCP_CONTENT_TYPE1; + + return 0; +} + static bool intel_hdcp_is_ksv_valid(u8 *ksv) { @@ -1474,13 +1532,14 @@ static int _hdcp2_propagate_stream_management_info(struct intel_connector *connector) { struct intel_digital_port *dig_port = intel_attached_dig_port(connector); + struct hdcp_port_data *data = _port->port_data; struct intel_hdcp *hdcp = >hdcp; union { struct hdcp2_rep_stream_manage stream_manage; struct hdcp2_rep_stream_ready stream_ready; } msgs; const struct intel_hdcp_shim *shim = hdcp->shim; - int ret; + int ret, streams_size_delta, i; if (connector->hdcp.seq_num_m > HDCP_2_2_SEQ_NUM_MAX) return -ERANGE; @@ -1489,16 +1548,18 @@ int _hdcp2_propagate_stream_management_info(struct intel_connector *connector) msgs.stream_manage.msg_id = HDCP_2_2_REP_STREAM_MANAGE; drm_hdcp_cpu_to_be24(msgs.stream_manage.seq_num_m, hdcp->seq_num_m); - /* K no of streams is fixed as 1. Stored as big-endian. */ - msgs.stream_manage.k = cpu_to_be16(1); + msgs.stream_manage.k = cpu_to_be16(data->k); - /* For HDMI this is forced to be 0x0. For DP SST also this is 0x0. */ -
[PATCH v4 15/16] drm/i915/hdcp: Support for HDCP 2.2 MST shim callbacks
Add support for HDCP 2.2 DP MST shim callback. This adds existing DP HDCP shim callback for Link Authentication and Encryption and HDCP 2.2 stream encryption callback. v2: Added a WARN_ON() instead of drm_err. [Uma] Cosmetic chnages. [Uma] Cc: Ramalingam C Signed-off-by: Anshuman Gupta --- .../drm/i915/display/intel_display_types.h| 4 + drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 80 +-- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index dfb5be64e03a..4cbb151ff3cf 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -374,6 +374,10 @@ struct intel_hdcp_shim { int (*config_stream_type)(struct intel_digital_port *dig_port, bool is_repeater, u8 type); + /* Enable/Disable HDCP 2.2 stream encryption on DP MST Transport Link */ + int (*stream_2_2_encryption)(struct intel_digital_port *dig_port, +bool enable); + /* HDCP2.2 Link Integrity Check */ int (*check_2_2_link)(struct intel_digital_port *dig_port, struct intel_connector *connector); diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c index 4be61e7fde4e..35c1543fe0e2 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c @@ -698,18 +698,14 @@ intel_dp_mst_hdcp_stream_encryption(struct intel_digital_port *dig_port, return 0; } -static -bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port, - struct intel_connector *connector) +static bool intel_dp_mst_get_qses_status(struct intel_digital_port *dig_port, +struct intel_connector *connector) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); - struct intel_dp *intel_dp = _port->dp; struct drm_dp_query_stream_enc_status_ack_reply reply; + struct intel_dp *intel_dp = _port->dp; int ret; - if (!intel_dp_hdcp_check_link(dig_port, connector)) - return false; - ret = drm_dp_send_query_stream_enc_status(_dp->mst_mgr, connector->port, ); if (ret) { @@ -722,6 +718,69 @@ bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port, return reply.auth_completed && reply.encryption_enabled; } +static +bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port, + struct intel_connector *connector) +{ + if (!intel_dp_hdcp_check_link(dig_port, connector)) + return false; + + return intel_dp_mst_get_qses_status(dig_port, connector); +} + +static int +intel_dp_mst_hdcp2_stream_encryption(struct intel_digital_port *dig_port, +bool enable) +{ + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); + struct hdcp_port_data *data = _port->port_data; + struct intel_dp *dp = _port->dp; + struct intel_hdcp *hdcp = >attached_connector->hdcp; + enum port port = dig_port->base.port; + /* HDCP2.x register uses stream transcoder */ + enum transcoder cpu_transcoder = hdcp->stream_transcoder; + int ret; + + drm_WARN_ON(>drm, enable && + !!(intel_de_read(i915, HDCP2_AUTH_STREAM(i915, cpu_transcoder, port)) + & AUTH_STREAM_TYPE) != data->streams[0].stream_type); + + ret = intel_dp_mst_toggle_hdcp_stream_select(dig_port, enable); + if (ret) + return ret; + + /* Wait for encryption confirmation */ + if (intel_de_wait_for_register(i915, + HDCP2_STREAM_STATUS(i915, cpu_transcoder, port), + STREAM_ENCRYPTION_STATUS, + enable ? STREAM_ENCRYPTION_STATUS : 0, + HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) { + drm_err(>drm, "Timed out waiting for stream encryption %s\n", + enable ? "enabled" : "disabled"); + return -ETIMEDOUT; + } + + return 0; +} + +/* + * DP v2.0 I.3.3 ignore the stream signature L' in QSES reply msg reply. + * I.3.5 MST source device may use a QSES msg to query downstream status + * for a particular stream. + */ +static +int intel_dp_mst_hdcp2_check_link(struct intel_digital_port *dig_port, + struct intel_connector *connector) +{ + int ret; + + ret = intel_dp_hdcp2_check_link(dig_port, connector); + if (ret) + return ret; + + return intel_dp_mst_get_qses_status(dig_port, connector) ? 0 : -EINVAL; +} +
[PATCH v4 07/16] drm/i915/hdcp: Enable Gen12 HDCP 1.4 DP MST support
Enable HDCP 1.4 over DP MST for Gen12. This also enable the stream encryption support for older generations, which was missing earlier. v2: - Added debug print for stream encryption. - Disable the hdcp on port after disabling last stream encryption. v3: - Cosmetic change, removed the value less comment. [Uma] Cc: Ramalingam C Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 10 ++--- drivers/gpu/drm/i915/display/intel_hdcp.c | 43 ++--- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 16865b200062..f00e12fc83e8 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -826,13 +826,9 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo intel_attach_force_audio_property(connector); intel_attach_broadcast_rgb_property(connector); - - /* TODO: Figure out how to make HDCP work on GEN12+ */ - if (INTEL_GEN(dev_priv) < 12) { - ret = intel_dp_init_hdcp(dig_port, intel_connector); - if (ret) - DRM_DEBUG_KMS("HDCP init failed, skipping.\n"); - } + ret = intel_dp_init_hdcp(dig_port, intel_connector); + if (ret) + drm_dbg_kms(_priv->drm, "HDCP init failed, skipping.\n"); /* * Reuse the prop from the SST connector because we're diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 0322a83c151d..937af4aeaac2 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -612,7 +612,12 @@ int intel_hdcp_auth_downstream(struct intel_connector *connector) return ret; } -/* Implements Part 1 of the HDCP authorization procedure */ +/* + * Implements Part 1 of the HDCP authorization procedure. + * Authentication Part 1 steps for Multi-stream DisplayPort. + * Step 1. Auth Part 1 sequence on the driving MST Trasport Link. + * Step 2. Enable encryption for each stream that requires encryption. + */ static int intel_hdcp_auth(struct intel_connector *connector) { struct intel_digital_port *dig_port = intel_attached_dig_port(connector); @@ -766,10 +771,16 @@ static int intel_hdcp_auth(struct intel_connector *connector) return -ETIMEDOUT; } - /* -* XXX: If we have MST-connected devices, we need to enable encryption -* on those as well. -*/ + /* DP MST Auth Part 1 Step 2.a and Step 2.b */ + if (shim->stream_encryption) { + ret = shim->stream_encryption(dig_port, true); + if (ret) { + drm_err(_priv->drm, "Failed to enable HDCP 1.4 stream enc\n"); + return ret; + } + drm_dbg_kms(_priv->drm, "HDCP 1.4 tras %s stream encrypted\n", + transcoder_name(hdcp->stream_transcoder)); + } if (repeater_present) return intel_hdcp_auth_downstream(connector); @@ -791,18 +802,22 @@ static int _intel_hdcp_disable(struct intel_connector *connector) drm_dbg_kms(_priv->drm, "[%s:%d] HDCP is being disabled...\n", connector->base.name, connector->base.base.id); + if (hdcp->shim->stream_encryption) { + ret = hdcp->shim->stream_encryption(dig_port, false); + if (ret) { + drm_err(_priv->drm, "Failed to disable HDCP 1.4 stream enc\n"); + return ret; + } + drm_dbg_kms(_priv->drm, "HDCP 1.4 trans %s stream encryption disabled\n", + transcoder_name(hdcp->stream_transcoder)); + } + /* -* If there are other connectors on this port using HDCP, don't disable -* it. Instead, toggle the HDCP signalling off on that particular -* connector/pipe and exit. +* If there are other connectors on this port using HDCP, don't disable it. +* Repeat steps 1-2 for each stream that no longer requires encryption. */ - if (dig_port->num_hdcp_streams > 0) { - ret = hdcp->shim->toggle_signalling(dig_port, - cpu_transcoder, false); - if (ret) - DRM_ERROR("Failed to disable HDCP signalling\n"); + if (dig_port->num_hdcp_streams > 0) return ret; - } hdcp->hdcp_encrypted = false; intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, port), 0); -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 09/16] drm/i915/hdcp: Encapsulate hdcp_port_data to dig_port
hdcp_port_data is specific to a port on which HDCP encryption is getting enabled, so encapsulate it to intel_digital_port. This will be required to enable HDCP 2.2 stream encryption. Cc: Ramalingam C Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_ddi.c | 2 + .../drm/i915/display/intel_display_types.h| 5 +- drivers/gpu/drm/i915/display/intel_hdcp.c | 56 +++ 3 files changed, 39 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 779603a38cfc..1bc6cf0b83ec 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -4746,6 +4746,8 @@ static void intel_ddi_encoder_destroy(struct drm_encoder *encoder) intel_dp_encoder_flush_work(encoder); drm_encoder_cleanup(encoder); + if (dig_port) + kfree(dig_port->port_data.streams); kfree(dig_port); } diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 59b8fc21e3e8..749c3a7e0b45 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -402,7 +402,6 @@ struct intel_hdcp { * content can flow only through a link protected by HDCP2.2. */ u8 content_type; - struct hdcp_port_data port_data; bool is_paired; bool is_repeater; @@ -1446,10 +1445,12 @@ struct intel_digital_port { enum phy_fia tc_phy_fia; u8 tc_phy_fia_idx; - /* protects num_hdcp_streams reference count */ + /* protects num_hdcp_streams reference count, port_data */ struct mutex hdcp_mutex; /* the number of pipes using HDCP signalling out of this port */ unsigned int num_hdcp_streams; + /* HDCP port data need to pass to security f/w */ + struct hdcp_port_data port_data; void (*write_infoframe)(struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state, diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index b0f47687bc59..a5ec4f72f50f 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -15,6 +15,7 @@ #include #include +#include "i915_drv.h" #include "i915_reg.h" #include "intel_display_power.h" #include "intel_display_types.h" @@ -1028,7 +1029,8 @@ static int hdcp2_prepare_ake_init(struct intel_connector *connector, struct hdcp2_ake_init *ake_data) { - struct hdcp_port_data *data = >hdcp.port_data; + struct intel_digital_port *dig_port = intel_attached_dig_port(connector); + struct hdcp_port_data *data = _port->port_data; struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct i915_hdcp_comp_master *comp; int ret; @@ -1057,7 +1059,8 @@ hdcp2_verify_rx_cert_prepare_km(struct intel_connector *connector, struct hdcp2_ake_no_stored_km *ek_pub_km, size_t *msg_sz) { - struct hdcp_port_data *data = >hdcp.port_data; + struct intel_digital_port *dig_port = intel_attached_dig_port(connector); + struct hdcp_port_data *data = _port->port_data; struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct i915_hdcp_comp_master *comp; int ret; @@ -1084,7 +1087,8 @@ hdcp2_verify_rx_cert_prepare_km(struct intel_connector *connector, static int hdcp2_verify_hprime(struct intel_connector *connector, struct hdcp2_ake_send_hprime *rx_hprime) { - struct hdcp_port_data *data = >hdcp.port_data; + struct intel_digital_port *dig_port = intel_attached_dig_port(connector); + struct hdcp_port_data *data = _port->port_data; struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct i915_hdcp_comp_master *comp; int ret; @@ -1109,7 +1113,8 @@ static int hdcp2_store_pairing_info(struct intel_connector *connector, struct hdcp2_ake_send_pairing_info *pairing_info) { - struct hdcp_port_data *data = >hdcp.port_data; + struct intel_digital_port *dig_port = intel_attached_dig_port(connector); + struct hdcp_port_data *data = _port->port_data; struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct i915_hdcp_comp_master *comp; int ret; @@ -1135,7 +1140,8 @@ static int hdcp2_prepare_lc_init(struct intel_connector *connector, struct hdcp2_lc_init *lc_init) { - struct hdcp_port_data *data = >hdcp.port_data; + struct intel_digital_port *dig_port = intel_attached_dig_port(connector); + struct hdcp_port_data *data = _port->port_data; struct drm_i915_private
[PATCH v4 06/16] drm/i915/hdcp: HDCP stream encryption support
Both HDCP_{1.x,2.x} requires to select/deselect Multistream HDCP bit in TRANS_DDI_FUNC_CTL in order to enable/disable stream HDCP encryption over DP MST Transport Link. HDCP 1.4 stream encryption requires to validate the stream encryption status in HDCP_STATUS_{TRANSCODER,PORT} register driving that link in order to enable/disable the stream encryption. Both of above requirement are same for all Gen with respect to B.Spec Documentation. v2: Cosmetic changes function name, error msg print and stream typo fixes. [Uma] Cc: Ramalingam C Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_ddi.c | 10 +-- drivers/gpu/drm/i915/display/intel_ddi.h | 6 +- .../drm/i915/display/intel_display_types.h| 4 + drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 80 --- drivers/gpu/drm/i915/display/intel_hdmi.c | 14 ++-- drivers/gpu/drm/i915/i915_reg.h | 1 + 6 files changed, 90 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 9fce623e951e..779603a38cfc 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -1948,9 +1948,9 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state } } -int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder, -enum transcoder cpu_transcoder, -bool enable) +int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder, + enum transcoder cpu_transcoder, + bool enable, u32 hdcp_mask) { struct drm_device *dev = intel_encoder->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -1965,9 +1965,9 @@ int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder, tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder)); if (enable) - tmp |= TRANS_DDI_HDCP_SIGNALLING; + tmp |= hdcp_mask; else - tmp &= ~TRANS_DDI_HDCP_SIGNALLING; + tmp &= ~hdcp_mask; intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder), tmp); intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref); return ret; diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h b/drivers/gpu/drm/i915/display/intel_ddi.h index dcc711cfe4fe..a4dd815c 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.h +++ b/drivers/gpu/drm/i915/display/intel_ddi.h @@ -50,9 +50,9 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state); u32 ddi_signal_levels(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state); -int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder, -enum transcoder cpu_transcoder, -bool enable); +int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder, + enum transcoder cpu_transcoder, + bool enable, u32 hdcp_mask); void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder); #endif /* __INTEL_DDI_H__ */ diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index c47124a679b6..59b8fc21e3e8 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -339,6 +339,10 @@ struct intel_hdcp_shim { enum transcoder cpu_transcoder, bool enable); + /* Enable/Disable stream encryption on DP MST Transport Link */ + int (*stream_encryption)(struct intel_digital_port *dig_port, +bool enable); + /* Ensures the link is still protected */ bool (*check_link)(struct intel_digital_port *dig_port, struct intel_connector *connector); diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c index 03424d20e9f7..6dcbfaffd2c5 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c @@ -16,6 +16,30 @@ #include "intel_dp.h" #include "intel_hdcp.h" +static unsigned int transcoder_to_stream_enc_status(enum transcoder cpu_transcoder) +{ + u32 stream_enc_mask; + + switch (cpu_transcoder) { + case TRANSCODER_A: + stream_enc_mask = HDCP_STATUS_STREAM_A_ENC; + break; + case TRANSCODER_B: + stream_enc_mask = HDCP_STATUS_STREAM_B_ENC; + break; + case TRANSCODER_C: + stream_enc_mask = HDCP_STATUS_STREAM_C_ENC; + break; + case TRANSCODER_D: +
[PATCH v4 11/16] drm/hdcp: Max MST content streams
Let's define Maximum MST content streams up to four generically which can be supported by modern display controllers. Cc: Sean Paul Cc: Ramalingam C Acked-by: Maarten Lankhorst Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- include/drm/drm_hdcp.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h index fe58dbb46962..ac22c246542a 100644 --- a/include/drm/drm_hdcp.h +++ b/include/drm/drm_hdcp.h @@ -101,11 +101,11 @@ /* Following Macros take a byte at a time for bit(s) masking */ /* - * TODO: This has to be changed for DP MST, as multiple stream on - * same port is possible. - * For HDCP2.2 on HDMI and DP SST this value is always 1. + * TODO: HDCP_2_2_MAX_CONTENT_STREAMS_CNT is based upon actual + * H/W MST streams capacity. + * This required to be moved out to platform specific header. */ -#define HDCP_2_2_MAX_CONTENT_STREAMS_CNT 1 +#define HDCP_2_2_MAX_CONTENT_STREAMS_CNT 4 #define HDCP_2_2_TXCAP_MASK_LEN2 #define HDCP_2_2_RXCAPS_LEN3 #define HDCP_2_2_RX_REPEATER(x)((x) & BIT(0)) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 13/16] drm/i915/hdcp: Pass connector to check_2_2_link
This requires for HDCP 2.2 MST check link. Cc: Ramalingam C Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_display_types.h | 3 ++- drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 3 ++- drivers/gpu/drm/i915/display/intel_hdcp.c | 2 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 24e0067c2e7c..dfb5be64e03a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -375,7 +375,8 @@ struct intel_hdcp_shim { bool is_repeater, u8 type); /* HDCP2.2 Link Integrity Check */ - int (*check_2_2_link)(struct intel_digital_port *dig_port); + int (*check_2_2_link)(struct intel_digital_port *dig_port, + struct intel_connector *connector); }; struct intel_hdcp { diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c index 591b68e5de48..4be61e7fde4e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c @@ -585,7 +585,8 @@ int intel_dp_hdcp2_config_stream_type(struct intel_digital_port *dig_port, } static -int intel_dp_hdcp2_check_link(struct intel_digital_port *dig_port) +int intel_dp_hdcp2_check_link(struct intel_digital_port *dig_port, + struct intel_connector *connector) { u8 rx_status; int ret; diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 1df6d4a23476..87f7aaf3a319 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1940,7 +1940,7 @@ static int intel_hdcp2_check_link(struct intel_connector *connector) goto out; } - ret = hdcp->shim->check_2_2_link(dig_port); + ret = hdcp->shim->check_2_2_link(dig_port, connector); if (ret == HDCP_LINK_PROTECTED) { if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) { intel_hdcp_update_value(connector, diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 0788de04711b..bd0d91101464 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -1734,7 +1734,8 @@ int intel_hdmi_hdcp2_read_msg(struct intel_digital_port *dig_port, } static -int intel_hdmi_hdcp2_check_link(struct intel_digital_port *dig_port) +int intel_hdmi_hdcp2_check_link(struct intel_digital_port *dig_port, + struct intel_connector *connector) { u8 rx_status[HDCP_2_2_HDMI_RXSTATUS_LEN]; int ret; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 08/16] drm/i915/hdcp: Pass dig_port to intel_hdcp_init
Pass dig_port as an argument to intel_hdcp_init() and intel_hdcp2_init(). This will be required for HDCP 2.2 stream encryption. Cc: Ramalingam C Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 4 ++-- drivers/gpu/drm/i915/display/intel_hdcp.c| 12 +++- drivers/gpu/drm/i915/display/intel_hdcp.h| 4 +++- drivers/gpu/drm/i915/display/intel_hdmi.c| 2 +- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c index 6dcbfaffd2c5..591b68e5de48 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c @@ -751,10 +751,10 @@ int intel_dp_init_hdcp(struct intel_digital_port *dig_port, return 0; if (intel_connector->mst_port) - return intel_hdcp_init(intel_connector, port, + return intel_hdcp_init(intel_connector, dig_port, _dp_mst_hdcp_shim); else if (!intel_dp_is_edp(intel_dp)) - return intel_hdcp_init(intel_connector, port, + return intel_hdcp_init(intel_connector, dig_port, _dp_hdcp_shim); return 0; diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 937af4aeaac2..b0f47687bc59 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1982,12 +1982,13 @@ static enum mei_fw_tc intel_get_mei_fw_tc(enum transcoder cpu_transcoder) } static int initialize_hdcp_port_data(struct intel_connector *connector, -enum port port, +struct intel_digital_port *dig_port, const struct intel_hdcp_shim *shim) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_hdcp *hdcp = >hdcp; struct hdcp_port_data *data = >port_data; + enum port port = dig_port->base.port; if (INTEL_GEN(dev_priv) < 12) data->fw_ddi = intel_get_mei_fw_ddi_index(port); @@ -2060,14 +2061,15 @@ void intel_hdcp_component_init(struct drm_i915_private *dev_priv) } } -static void intel_hdcp2_init(struct intel_connector *connector, enum port port, +static void intel_hdcp2_init(struct intel_connector *connector, +struct intel_digital_port *dig_port, const struct intel_hdcp_shim *shim) { struct drm_i915_private *i915 = to_i915(connector->base.dev); struct intel_hdcp *hdcp = >hdcp; int ret; - ret = initialize_hdcp_port_data(connector, port, shim); + ret = initialize_hdcp_port_data(connector, dig_port, shim); if (ret) { drm_dbg_kms(>drm, "Mei hdcp data init failed\n"); return; @@ -2077,7 +2079,7 @@ static void intel_hdcp2_init(struct intel_connector *connector, enum port port, } int intel_hdcp_init(struct intel_connector *connector, - enum port port, + struct intel_digital_port *dig_port, const struct intel_hdcp_shim *shim) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -2088,7 +2090,7 @@ int intel_hdcp_init(struct intel_connector *connector, return -EINVAL; if (is_hdcp2_supported(dev_priv) && !connector->mst_port) - intel_hdcp2_init(connector, port, shim); + intel_hdcp2_init(connector, dig_port, shim); ret = drm_connector_attach_content_protection_property(>base, diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h index b912a3a0f5b8..8f53b0c7fe5c 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.h +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h @@ -18,13 +18,15 @@ struct intel_connector; struct intel_crtc_state; struct intel_encoder; struct intel_hdcp_shim; +struct intel_digital_port; enum port; enum transcoder; void intel_hdcp_atomic_check(struct drm_connector *connector, struct drm_connector_state *old_state, struct drm_connector_state *new_state); -int intel_hdcp_init(struct intel_connector *connector, enum port port, +int intel_hdcp_init(struct intel_connector *connector, + struct intel_digital_port *dig_port, const struct intel_hdcp_shim *hdcp_shim); int intel_hdcp_enable(struct intel_connector *connector, const struct intel_crtc_state *pipe_config, u8 content_type); diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index f58469226694..0788de04711b 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++
[PATCH v4 10/16] misc/mei/hdcp: Fix AUTH_STREAM_REQ cmd buffer len
Fix the size of WIRED_REPEATER_AUTH_STREAM_REQ cmd buffer size. It is based upon the actual number of MST streams and size of wired_cmd_repeater_auth_stream_req_in. Excluding the size of hdcp_cmd_header. v2: hdcp_cmd_header size annotation nitpick. [Tomas] Cc: Tomas Winkler Cc: Ramalingam C Acked-by: Tomas Winkler Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- drivers/misc/mei/hdcp/mei_hdcp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c index 9ae9669e46ea..3506a3534294 100644 --- a/drivers/misc/mei/hdcp/mei_hdcp.c +++ b/drivers/misc/mei/hdcp/mei_hdcp.c @@ -569,8 +569,7 @@ static int mei_hdcp_verify_mprime(struct device *dev, verify_mprime_in->header.api_version = HDCP_API_VERSION; verify_mprime_in->header.command_id = WIRED_REPEATER_AUTH_STREAM_REQ; verify_mprime_in->header.status = ME_HDCP_STATUS_SUCCESS; - verify_mprime_in->header.buffer_len = - WIRED_CMD_BUF_LEN_REPEATER_AUTH_STREAM_REQ_MIN_IN; + verify_mprime_in->header.buffer_len = cmd_size - sizeof(verify_mprime_in->header); verify_mprime_in->port.integrated_port_type = data->port_type; verify_mprime_in->port.physical_port = (u8)data->fw_ddi; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 14/16] drm/i915/hdcp: Add HDCP 2.2 stream register
Add HDCP 2.2 DP MST HDCP2_STREAM_STATUS and HDCP2_AUTH_STREAM register in i915_reg header. Cc: Ramalingam C Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/i915_reg.h | 30 ++ 1 file changed, 30 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 77461cde6549..c9678c77883d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9894,6 +9894,7 @@ enum skl_power_gate { _PORTD_HDCP2_BASE, \ _PORTE_HDCP2_BASE, \ _PORTF_HDCP2_BASE) + (x)) + #define PORT_HDCP2_AUTH(port) _PORT_HDCP2_BASE(port, 0x98) #define _TRANSA_HDCP2_AUTH 0x66498 #define _TRANSB_HDCP2_AUTH 0x66598 @@ -9933,6 +9934,35 @@ enum skl_power_gate { TRANS_HDCP2_STATUS(trans) : \ PORT_HDCP2_STATUS(port)) +#define PORT_HDCP2_STREAM_STATUS(port) _PORT_HDCP2_BASE(port, 0xC0) +#define _TRANSA_HDCP2_STREAM_STATUS0x664C0 +#define _TRANSB_HDCP2_STREAM_STATUS0x665C0 +#define TRANS_HDCP2_STREAM_STATUS(trans) _MMIO_TRANS(trans, \ + _TRANSA_HDCP2_STREAM_STATUS, \ + _TRANSB_HDCP2_STREAM_STATUS) +#define STREAM_ENCRYPTION_STATUS BIT(31) +#define STREAM_TYPE_STATUS BIT(30) +#define HDCP2_STREAM_STATUS(dev_priv, trans, port) \ + (INTEL_GEN(dev_priv) >= 12 ? \ +TRANS_HDCP2_STREAM_STATUS(trans) : \ +PORT_HDCP2_STREAM_STATUS(port)) + +#define _PORTA_HDCP2_AUTH_STREAM 0x66F00 +#define _PORTB_HDCP2_AUTH_STREAM 0x66F04 +#define PORT_HDCP2_AUTH_STREAM(port) _MMIO_PORT(port, \ + _PORTA_HDCP2_AUTH_STREAM, \ + _PORTB_HDCP2_AUTH_STREAM) +#define _TRANSA_HDCP2_AUTH_STREAM 0x66F00 +#define _TRANSB_HDCP2_AUTH_STREAM 0x66F04 +#define TRANS_HDCP2_AUTH_STREAM(trans) _MMIO_TRANS(trans, \ + _TRANSA_HDCP2_AUTH_STREAM, \ + _TRANSB_HDCP2_AUTH_STREAM) +#define AUTH_STREAM_TYPE BIT(31) +#define HDCP2_AUTH_STREAM(dev_priv, trans, port) \ + (INTEL_GEN(dev_priv) >= 12 ? \ +TRANS_HDCP2_AUTH_STREAM(trans) : \ +PORT_HDCP2_AUTH_STREAM(port)) + /* Per-pipe DDI Function Control */ #define _TRANS_DDI_FUNC_CTL_A 0x60400 #define _TRANS_DDI_FUNC_CTL_B 0x61400 -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 16/16] drm/i915/hdcp: Enable HDCP 2.2 MST support
Enable HDCP 2.2 over DP MST. Authenticate and enable port encryption only once for an active HDCP 2.2 session, once port is authenticated and encrypted enable encryption for each stream that requires encryption on this port. Similarly disable the stream encryption for each encrypted stream, once all encrypted stream encryption is disabled, disable the port HDCP encryption and deauthenticate the port. Cc: Ramalingam C Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_hdcp.c | 46 ++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 87f7aaf3a319..71fd01bf63a6 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1693,6 +1693,32 @@ static int hdcp2_authenticate_sink(struct intel_connector *connector) return ret; } +static int hdcp2_enable_stream_encryption(struct intel_connector *connector) +{ + struct intel_digital_port *dig_port = intel_attached_dig_port(connector); + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct intel_hdcp *hdcp = >hdcp; + enum transcoder cpu_transcoder = hdcp->cpu_transcoder; + enum port port = dig_port->base.port; + int ret = 0; + + if (!(intel_de_read(dev_priv, HDCP2_STATUS(dev_priv, cpu_transcoder, port)) & + LINK_ENCRYPTION_STATUS)) { + drm_err(_priv->drm, "HDCP 2.2 Link is not encrypted\n"); + return -EPERM; + } + + if (hdcp->shim->stream_2_2_encryption) { + ret = hdcp->shim->stream_2_2_encryption(dig_port, true); + if (ret) { + drm_err(_priv->drm, "Failed to enable HDCP 2.2 stream enc\n"); + return ret; + } + } + + return ret; +} + static int hdcp2_enable_encryption(struct intel_connector *connector) { struct intel_digital_port *dig_port = intel_attached_dig_port(connector); @@ -1831,7 +1857,7 @@ static int hdcp2_authenticate_and_encrypt(struct intel_connector *connector) drm_dbg_kms(>drm, "Port deauth failed.\n"); } - if (!ret) { + if (!ret && !dig_port->port_auth) { /* * Ensuring the required 200mSec min time interval between * Session Key Exchange and encryption. @@ -1846,6 +1872,8 @@ static int hdcp2_authenticate_and_encrypt(struct intel_connector *connector) } } + ret = hdcp2_enable_stream_encryption(connector); + return ret; } @@ -1891,11 +1919,23 @@ static int _intel_hdcp2_disable(struct intel_connector *connector) struct intel_digital_port *dig_port = intel_attached_dig_port(connector); struct drm_i915_private *i915 = to_i915(connector->base.dev); struct hdcp_port_data *data = _port->port_data; + struct intel_hdcp *hdcp = >hdcp; int ret; drm_dbg_kms(>drm, "[%s:%d] HDCP2.2 is being Disabled\n", connector->base.name, connector->base.base.id); + if (hdcp->shim->stream_2_2_encryption) { + ret = hdcp->shim->stream_2_2_encryption(dig_port, false); + if (ret) { + drm_err(>drm, "Failed to disable HDCP 2.2 stream enc\n"); + return ret; + } + } + + if (dig_port->num_hdcp_streams > 0) + return ret; + ret = hdcp2_disable_encryption(connector); if (hdcp2_deauthenticate_port(connector) < 0) @@ -1919,6 +1959,7 @@ static int intel_hdcp2_check_link(struct intel_connector *connector) int ret = 0; mutex_lock(>mutex); + mutex_lock(_port->hdcp_mutex); cpu_transcoder = hdcp->cpu_transcoder; /* hdcp2_check_link is expected only when HDCP2.2 is Enabled */ @@ -1996,6 +2037,7 @@ static int intel_hdcp2_check_link(struct intel_connector *connector) } out: + mutex_unlock(_port->hdcp_mutex); mutex_unlock(>mutex); return ret; } @@ -2177,7 +2219,7 @@ int intel_hdcp_init(struct intel_connector *connector, if (!shim) return -EINVAL; - if (is_hdcp2_supported(dev_priv) && !connector->mst_port) + if (is_hdcp2_supported(dev_priv)) intel_hdcp2_init(connector, dig_port, shim); ret = -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 05/16] drm/i915/hdcp: Move HDCP enc status timeout to header
DP MST stream encryption status requires time of a link frame in order to change its status, but as there were some HDCP encryption timeout observed earlier, it is safer to use ENCRYPT_STATUS_CHANGE_TIMEOUT_MS timeout for stream status too, it requires to move the macro to a header. It will be used by both HDCP{1.x,2.x} stream status timeout. Related: 'commit 7e90e8d0c0ea ("drm/i915: Increase timeout for Encrypt status change")' Cc: Ramalingam C Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_hdcp.c | 9 - drivers/gpu/drm/i915/display/intel_hdcp.h | 2 ++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index fc5de48456ad..0322a83c151d 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -23,7 +23,6 @@ #include "intel_connector.h" #define KEY_LOAD_TRIES 5 -#define ENCRYPT_STATUS_CHANGE_TIMEOUT_MS 50 #define HDCP2_LC_RETRY_CNT 3 static @@ -762,7 +761,7 @@ static int intel_hdcp_auth(struct intel_connector *connector) if (intel_de_wait_for_set(dev_priv, HDCP_STATUS(dev_priv, cpu_transcoder, port), HDCP_STATUS_ENC, - ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) { + HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) { drm_err(_priv->drm, "Timed out waiting for encryption\n"); return -ETIMEDOUT; } @@ -809,7 +808,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector) intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, port), 0); if (intel_de_wait_for_clear(dev_priv, HDCP_STATUS(dev_priv, cpu_transcoder, port), - ~0, ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) { + ~0, HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS)) { drm_err(_priv->drm, "Failed to disable HDCP, timeout clearing status\n"); return -ETIMEDOUT; @@ -1641,7 +1640,7 @@ static int hdcp2_enable_encryption(struct intel_connector *connector) HDCP2_STATUS(dev_priv, cpu_transcoder, port), LINK_ENCRYPTION_STATUS, - ENCRYPT_STATUS_CHANGE_TIMEOUT_MS); + HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS); return ret; } @@ -1665,7 +1664,7 @@ static int hdcp2_disable_encryption(struct intel_connector *connector) HDCP2_STATUS(dev_priv, cpu_transcoder, port), LINK_ENCRYPTION_STATUS, - ENCRYPT_STATUS_CHANGE_TIMEOUT_MS); + HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS); if (ret == -ETIMEDOUT) drm_dbg_kms(_priv->drm, "Disable Encryption Timedout"); diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h index bc51c1e9b481..b912a3a0f5b8 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.h +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h @@ -8,6 +8,8 @@ #include +#define HDCP_ENCRYPT_STATUS_CHANGE_TIMEOUT_MS 50 + struct drm_connector; struct drm_connector_state; struct drm_i915_private; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 00/16] HDCP 2.2 and HDCP 1.4 Gen12 DP MST support
This is v4 version to test with IGT https://patchwork.freedesktop.org/series/82987/ This has addressed the review comments from Uma. It has been also tested manually with IGT above series. [PATCH v4 10/16] misc/mei/hdcp: Fix AUTH_STREAM_REQ cmd buffer len has an Ack from Tomas to merge it via drm-intel. [PATCH v4 11/16] drm/hdcp: Max MST content streams has an Ack from drm-misc maintainer to merge it via dm-intel. Test-with: 20201023100709.5211-2-karthik@intel.com Anshuman Gupta (16): drm/i915/hdcp: Update CP property in update_pipe drm/i915/hdcp: Get conn while content_type changed drm/i915/hotplug: Handle CP_IRQ for DP-MST drm/i915/hdcp: DP MST transcoder for link and stream drm/i915/hdcp: Move HDCP enc status timeout to header drm/i915/hdcp: HDCP stream encryption support drm/i915/hdcp: Enable Gen12 HDCP 1.4 DP MST support drm/i915/hdcp: Pass dig_port to intel_hdcp_init drm/i915/hdcp: Encapsulate hdcp_port_data to dig_port misc/mei/hdcp: Fix AUTH_STREAM_REQ cmd buffer len drm/hdcp: Max MST content streams drm/i915/hdcp: MST streams support in hdcp port_data drm/i915/hdcp: Pass connector to check_2_2_link drm/i915/hdcp: Add HDCP 2.2 stream register drm/i915/hdcp: Support for HDCP 2.2 MST shim callbacks drm/i915/hdcp: Enable HDCP 2.2 MST support drivers/gpu/drm/i915/display/intel_ddi.c | 14 +- drivers/gpu/drm/i915/display/intel_ddi.h | 6 +- .../drm/i915/display/intel_display_types.h| 20 +- drivers/gpu/drm/i915/display/intel_dp.c | 14 +- drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 167 -- drivers/gpu/drm/i915/display/intel_dp_mst.c | 12 +- drivers/gpu/drm/i915/display/intel_hdcp.c | 285 ++ drivers/gpu/drm/i915/display/intel_hdcp.h | 8 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 19 +- drivers/gpu/drm/i915/i915_reg.h | 31 ++ drivers/misc/mei/hdcp/mei_hdcp.c | 3 +- include/drm/drm_hdcp.h| 8 +- 12 files changed, 466 insertions(+), 121 deletions(-) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 03/16] drm/i915/hotplug: Handle CP_IRQ for DP-MST
Handle CP_IRQ in DEVICE_SERVICE_IRQ_VECTOR_ESI0 It requires to call intel_hdcp_handle_cp_irq() in case of CP_IRQ is triggered by a sink in DP-MST topology. Cc: "Ville Syrjälä" Cc: Ramalingam C Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_dp.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 818daab252f3..21c6c9828cd7 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5657,6 +5657,17 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) "Could not write test response to sink\n"); } +static void +intel_dp_mst_hpd_irq(struct intel_dp *intel_dp, u8 *esi, bool *handled) +{ + drm_dp_mst_hpd_irq(_dp->mst_mgr, esi, handled); + + if (esi[1] & DP_CP_IRQ) { + intel_hdcp_handle_cp_irq(intel_dp->attached_connector); + *handled = true; + } +} + /** * intel_dp_check_mst_status - service any pending MST interrupts, check link status * @intel_dp: Intel DP struct @@ -5701,7 +5712,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp) drm_dbg_kms(>drm, "got esi %3ph\n", esi); - drm_dp_mst_hpd_irq(_dp->mst_mgr, esi, ); + intel_dp_mst_hpd_irq(intel_dp, esi, ); + if (!handled) break; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 04/16] drm/i915/hdcp: DP MST transcoder for link and stream
Gen12 has H/W delta with respect to HDCP{1.x,2.x} display engine instances lies in Transcoder instead of DDI as in Gen11. This requires hdcp driver to use mst_master_transcoder for link authentication and stream transcoder for stream encryption separately. This will be used for both HDCP 1.4 and HDCP 2.2 over DP MST on Gen12. Cc: Ramalingam C Reviewed-by: Uma Shankar Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_ddi.c | 2 +- .../gpu/drm/i915/display/intel_display_types.h| 2 ++ drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +- drivers/gpu/drm/i915/display/intel_hdcp.c | 15 +++ drivers/gpu/drm/i915/display/intel_hdcp.h | 2 +- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 63380b166c25..9fce623e951e 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -4059,7 +4059,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state, if (conn_state->content_protection == DRM_MODE_CONTENT_PROTECTION_DESIRED) intel_hdcp_enable(to_intel_connector(conn_state->connector), - crtc_state->cpu_transcoder, + crtc_state, (u8)conn_state->hdcp_content_type); } diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index f6f0626649e0..c47124a679b6 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -432,6 +432,8 @@ struct intel_hdcp { * Hence caching the transcoder here. */ enum transcoder cpu_transcoder; + /* Only used for DP MST stream encryption */ + enum transcoder stream_transcoder; }; struct intel_connector { diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index c8fcec4d0788..16865b200062 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -568,7 +568,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state, if (conn_state->content_protection == DRM_MODE_CONTENT_PROTECTION_DESIRED) intel_hdcp_enable(to_intel_connector(conn_state->connector), - pipe_config->cpu_transcoder, + pipe_config, (u8)conn_state->hdcp_content_type); } diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index b9d8825e2bb1..fc5de48456ad 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -2095,7 +2095,7 @@ int intel_hdcp_init(struct intel_connector *connector, } int intel_hdcp_enable(struct intel_connector *connector, - enum transcoder cpu_transcoder, u8 content_type) + const struct intel_crtc_state *pipe_config, u8 content_type) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_digital_port *dig_port = intel_attached_dig_port(connector); @@ -2111,10 +2111,17 @@ int intel_hdcp_enable(struct intel_connector *connector, drm_WARN_ON(_priv->drm, hdcp->value == DRM_MODE_CONTENT_PROTECTION_ENABLED); hdcp->content_type = content_type; - hdcp->cpu_transcoder = cpu_transcoder; + + if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST)) { + hdcp->cpu_transcoder = pipe_config->mst_master_transcoder; + hdcp->stream_transcoder = pipe_config->cpu_transcoder; + } else { + hdcp->cpu_transcoder = pipe_config->cpu_transcoder; + hdcp->stream_transcoder = INVALID_TRANSCODER; + } if (INTEL_GEN(dev_priv) >= 12) - hdcp->port_data.fw_tc = intel_get_mei_fw_tc(cpu_transcoder); + hdcp->port_data.fw_tc = intel_get_mei_fw_tc(hdcp->cpu_transcoder); /* * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup @@ -2234,7 +2241,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state, if (desired_and_not_enabled || content_protection_type_changed) intel_hdcp_enable(connector, - crtc_state->cpu_transcoder, + crtc_state, (u8)conn_state->hdcp_content_type); } diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h index 1bbf5b67ed0a..bc51c1e9b481 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.h +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h @@ -25,7 +25,7 @@ void intel_hdcp_atomic_check(struct drm_connector
[PATCH v4 01/16] drm/i915/hdcp: Update CP property in update_pipe
When crtc state need_modeset is true it is not necessary it is going to be a real modeset, it can turns to be a fastset instead of modeset. This turns content protection property to be DESIRED and hdcp update_pipe left with property to be in DESIRED state but actual hdcp->value was ENABLED. This issue is caught with DP MST setup, where we have multiple connector in same DP_MST topology. When disabling HDCP on one of DP MST connector leads to set the crtc state need_modeset to true for all other crtc driving the other DP-MST topology connectors. This turns up other DP MST connectors CP property to be DESIRED despite the actual hdcp->value is ENABLED. Above scenario fails the DP MST HDCP IGT test, disabling HDCP on one MST stream should not cause to disable HDCP on another MST stream on same DP MST topology. v2: Fix WARN_ON(connector->base.registration_state == DRM_CONNECTOR_REGISTERED) v3: Commit log improvement. [Uma] Added a comment before scheduling prop_work. [Uma] Fixes: 33f9a623bfc6 ("drm/i915/hdcp: Update CP as per the kernel internal state") Cc: Ramalingam C Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_hdcp.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index b2a4bbcfdcd2..eee8263405b9 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -2221,6 +2221,14 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state, desired_and_not_enabled = hdcp->value != DRM_MODE_CONTENT_PROTECTION_ENABLED; mutex_unlock(>mutex); + /* +* If HDCP already ENABLED and CP property is DESIRED, schedule +* prop_work to update correct CP property to user space. +*/ + if (!desired_and_not_enabled && !content_protection_type_changed) { + drm_connector_get(>base); + schedule_work(>prop_work); + } } if (desired_and_not_enabled || content_protection_type_changed) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 02/16] drm/i915/hdcp: Get conn while content_type changed
Get DRM connector reference count while scheduling a prop work to avoid any possible destroy of DRM connector when it is in DRM_CONNECTOR_REGISTERED state. Fixes: a6597faa2d59 ("drm/i915: Protect workers against disappearing connectors") Cc: Sean Paul Cc: Ramalingam C Signed-off-by: Anshuman Gupta --- drivers/gpu/drm/i915/display/intel_hdcp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index eee8263405b9..b9d8825e2bb1 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -2210,6 +2210,7 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state, if (content_protection_type_changed) { mutex_lock(>mutex); hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED; + drm_connector_get(>base); schedule_work(>prop_work); mutex_unlock(>mutex); } -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Following up
Hi Daniel, More about the 3 things we've discussed before: 1. Cleaning up con_font_op(): (drivers/tty/vt/vt.c) int con_font_op(struct vc_data *vc, struct console_font_op *op) { switch (op->op) { case KD_FONT_OP_SET: return con_font_set(vc, op); case KD_FONT_OP_GET: return con_font_get(vc, op); case KD_FONT_OP_SET_DEFAULT: return con_font_default(vc, op); case KD_FONT_OP_COPY: return con_font_copy(vc, op); } return -ENOSYS; } On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote: > I think if we change the conf_font_get/set/default/copy functions to not > take the *op struct (which is take pretty arbitrarily from one of the > ioctl), but the parameters each needs directly, that would clean up the > code a _lot_. This is on my TODO list! One day I came up with some idea about fbcon.c, so I postponed this a bit... 2. Removing dummy functions, like sisusbdummycon_font_set(): Turns out, before c396a5bf457f ("console: Expand dummy functions for CFI"), they were just some macros: -#define SISUSBCONDUMMY (void *)sisusbdummycon_dummy +static int sisusbdummycon_font_set(struct vc_data *vc, + struct console_font *font, + unsigned int flags) +{ + return 0; +} ...and they had been there for a very long (10+ years) time. Removing code like this makes me a bit nervous, and... On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote: > This actually does something. tbh I would not be surprises if the > fb_set utility is the only thing that uses this - with a bit of code > search we could perhaps confirm this, and delete all the other > implementations. ...you mentioned code search, where & what should we look at, in order to confirm it's safe to remove them? 3. Using `font_desc` in `vc_data`: Our plan for the gradual conversion was to use a helper function to set font for a vc, but after reviewing the 300-ish occurrence of `vc_font`, it seems like code doesn't usually set it as a whole: (drivers/usb/misc/sisusbvga/sisusb_con.c) [...] c->vc_font.height = sisusb->current_font_height; [...] ...that's it! It only cares about the height. There are only 4 or 5 places in fbcon.c that actually set all fields of `vc_font`, like: vc->vc_font.width = font->width; vc->vc_font.height = font->height; vc->vc_font.data = (void *)(p->fontdata = font->data); vc->vc_font.charcount = 256; /* FIXME Need to support more fonts */ To make it even more complicated, `p` is a `struct fbcon_display *`, containing yet another font data pointer (`fontdata`) that I think should be replaced by a `font_desc *`... In conclusion, I think it's all about a few hard problems in fbcon.c. I'll keep trying and see how it goes. Thank you, Peilin Ye ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/8] vhost: vringh: use krealloc_array()
On Tue, 2020-10-27 at 11:28 -0400, Michael S. Tsirkin wrote: > On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski > > > > Use the helper that checks for overflows internally instead of manually > > calculating the size of the new array. > > > > Signed-off-by: Bartosz Golaszewski > > No problem with the patch, it does introduce some symmetry in the code. Perhaps more symmetry by using kmemdup --- drivers/vhost/vringh.c | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c index 8bd8b403f087..99222a3651cd 100644 --- a/drivers/vhost/vringh.c +++ b/drivers/vhost/vringh.c @@ -191,26 +191,23 @@ static int move_to_indirect(const struct vringh *vrh, static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp) { struct kvec *new; - unsigned int flag, new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2; + size_t new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2; + size_t size; if (new_num < 8) new_num = 8; - flag = (iov->max_num & VRINGH_IOV_ALLOCATED); - if (flag) - new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp); - else { - new = kmalloc_array(new_num, sizeof(struct iovec), gfp); - if (new) { - memcpy(new, iov->iov, - iov->max_num * sizeof(struct iovec)); - flag = VRINGH_IOV_ALLOCATED; - } - } + if (unlikely(check_mul_overflow(new_num, sizeof(struct iovec), ))) + return -ENOMEM; + + if (iov->max_num & VRINGH_IOV_ALLOCATED) + new = krealloc(iov->iov, size, gfp); + else + new = kmemdup(iov->iov, size, gfp); if (!new) return -ENOMEM; iov->iov = new; - iov->max_num = (new_num | flag); + iov->max_num = new_num | VRINGH_IOV_ALLOCATED; return 0; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/gma500: avoid Woverride-init warning
On Tue, Oct 27, 2020 at 10:54 AM Patrik Jakobsson wrote: > On Tue, Oct 27, 2020 at 10:33 AM Daniel Vetter wrote: > > On Mon, Oct 26, 2020 at 08:41:04PM +0100, Arnd Bergmann wrote: > > > From: Arnd Bergmann > > > > > > gcc -Wextra notices that one of the fields in psbfb_roll_ops has two > > > initializers: > > > > > > drivers/gpu/drm/gma500/framebuffer.c:185:20: warning: initialized field > > > overwritten [-Woverride-init] > > > > > > Open-code this instead, leaving out the extraneous initializers for > > > .fb_pan_display. > > > > > > Fixes: 3da6c2f3b730 ("drm/gma500: use DRM_FB_HELPER_DEFAULT_OPS for > > > fb_ops") > > > Signed-off-by: Arnd Bergmann > > > > Scrollback is dead, so I'm not sure it's even worth to keep all this. I'd > > just garbage-collect this, maybe als the entire accelerator code and just > > leave psbfb_unaccel_ops behind ... > > -Daniel > > That's been my idea for quite some time. The gtt roll code is also > broken in multi display setups. > > Arnd, I can take care of this unless you feel an urge to do it yourself. That would be good, thanks I have no specific interest in the drm drivers, this is just part of a larger work to enable more of the W=1 options across the kernel by default, after all the existing warnings are addressed. Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/tilcdc: avoid 'make W=2' build failure
On Tue, Oct 27, 2020 at 4:31 PM Jyri Sarha wrote: > On 26/10/2020 21:41, Arnd Bergmann wrote: > > From: Arnd Bergmann > > > > The -Wmissing-field-initializer warning when building with W=2 > > turns into an error because tilcdc is built with -Werror: > > > > drm/tilcdc/tilcdc_drv.c:431:33: error: missing field 'data' initializer > > [-Werror,-Wmissing-field-initializers] { "regs", tilcdc_regs_show, 0 }, > > drm/tilcdc/tilcdc_drv.c:432:33: error: missing field 'data' initializer > > [-Werror,-Wmissing-field-initializers] { "mm", tilcdc_mm_show, 0 }, > > > > Add the missing field initializers to address the warning. > > > > Signed-off-by: Arnd Bergmann > > Reviewed-by: Jyri Sarha Thanks > Please let me know if you want me to merge this. Yes, it would be good if you can merge it directly. Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm: panel: simple: Allow timing constraints, not fixed delays
The simple panel code currently allows panels to define fixed delays at certain stages of initialization. These work OK, but they don't really map all that clearly to the requirements presented in many panel datasheets. Instead of defining a fixed delay, those datasheets provide a timing diagram and specify a minimum amount of time that needs to pass from event A to event B. Because of the way things are currently defined, most panels end up over-delaying. One prime example here is that a number of panels I've looked at define the amount of time that must pass between turning a panel off and turning it back on again. Since there is no way to specify this, many developers have listed this as the "unprepare" delay. However, if nobody ever tried to turn the panel on again in the next 500 ms (or whatever the delay was) then this delay was pointless. It's better to do the delay only in the case that someone tried to turn the panel on too quickly. Let's support specifying delays as constraints. We'll start with the one above and also a second one: the minimum time between prepare being done and doing the enable. On the panel I'm looking at, there's an 80 ms minimum time between HPD being asserted by the panel and setting the backlight enable GPIO. By specifying as a constraint we can enforce this without over-delaying. Specifically the link training is allowed to happen in parallel with this delay so adding a fixed 80 ms delay isn't ideal. Signed-off-by: Douglas Anderson --- drivers/gpu/drm/panel/panel-simple.c | 51 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 2be358fb46f7..cbbe71a2a940 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -92,6 +92,19 @@ struct panel_desc { unsigned int unprepare; } delay; + /** +* @prepare_to_enable_ms: If this many milliseconds hasn't passed after +*prepare finished, add a delay to the start +*of enable. +* @unprepare_to_prepare_ms: If this many milliseconds hasn't passed +* unprepare finished, add a delay to the +* start of prepare. +*/ + struct { + unsigned int prepare_to_enable_ms; + unsigned int unprepare_to_prepare_ms; + } timing_constraints; + u32 bus_format; u32 bus_flags; int connector_type; @@ -99,10 +112,12 @@ struct panel_desc { struct panel_simple { struct drm_panel base; - bool prepared; bool enabled; bool no_hpd; + ktime_t prepared_time; + ktime_t unprepared_time; + const struct panel_desc *desc; struct regulator *supply; @@ -230,6 +245,21 @@ static int panel_simple_get_non_edid_modes(struct panel_simple *panel, return num; } +static void panel_simple_enforce_constraint(ktime_t start_ktime, + unsigned int min_ms) +{ + ktime_t now_ktime, min_ktime; + + if (!min_ms) + return; + + min_ktime = ktime_add(start_ktime, ms_to_ktime(min_ms)); + now_ktime = ktime_get(); + + if (ktime_before(now_ktime, min_ktime)) + msleep(ktime_to_ms(ktime_sub(min_ktime, now_ktime)) + 1); +} + static int panel_simple_disable(struct drm_panel *panel) { struct panel_simple *p = to_panel_simple(panel); @@ -249,18 +279,19 @@ static int panel_simple_unprepare(struct drm_panel *panel) { struct panel_simple *p = to_panel_simple(panel); - if (!p->prepared) + if (!p->prepared_time) return 0; gpiod_set_value_cansleep(p->enable_gpio, 0); regulator_disable(p->supply); + p->prepared_time = 0; + p->unprepared_time = ktime_get(); + if (p->desc->delay.unprepare) msleep(p->desc->delay.unprepare); - p->prepared = false; - return 0; } @@ -296,9 +327,12 @@ static int panel_simple_prepare(struct drm_panel *panel) int err; int hpd_asserted; - if (p->prepared) + if (p->prepared_time) return 0; + panel_simple_enforce_constraint(p->unprepared_time, + p->desc->timing_constraints.unprepare_to_prepare_ms); + err = regulator_enable(p->supply); if (err < 0) { dev_err(panel->dev, "failed to enable supply: %d\n", err); @@ -333,7 +367,7 @@ static int panel_simple_prepare(struct drm_panel *panel) } } - p->prepared = true; + p->prepared_time = ktime_get(); return 0; } @@ -348,6 +382,9 @@ static int panel_simple_enable(struct drm_panel *panel) if (p->desc->delay.enable)
[PATCH 2/3] drm: panel: simple: Add BOE NV110WTM-N61
Add support for the BOE NV110WTM-N61 panel. The EDID lists two modes (one for 60 Hz refresh rate and one for 40 Hz), so we'll list both of them here. Note that the panel datasheet requires 80 ms between HPD asserting and the backlight power being turned on. We'll use the new timing constraints structure to do this cleanly. This assumes that the backlight will be enabled _after_ the panel enable finishes. This is how it works today and seems a sane assumption. Signed-off-by: Douglas Anderson --- drivers/gpu/drm/panel/panel-simple.c | 48 1 file changed, 48 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index cbbe71a2a940..c17c1203734a 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -1353,6 +1353,51 @@ static const struct panel_desc boe_nv101wxmn51 = { }, }; +static const struct drm_display_mode boe_nv110wtm_n61_modes[] = { + { + .clock = 207800, + .hdisplay = 2160, + .hsync_start = 2160 + 48, + .hsync_end = 2160 + 48 + 32, + .htotal = 2160 + 48 + 32 + 100, + .vdisplay = 1440, + .vsync_start = 1440 + 3, + .vsync_end = 1440 + 3 + 6, + .vtotal = 1440 + 3 + 6 + 31, + }, + { + .clock = 138500, + .hdisplay = 2160, + .hsync_start = 2160 + 48, + .hsync_end = 2160 + 48 + 32, + .htotal = 2160 + 48 + 32 + 100, + .vdisplay = 1440, + .vsync_start = 1440 + 3, + .vsync_end = 1440 + 3 + 6, + .vtotal = 1440 + 3 + 6 + 31, + }, +}; + +static const struct panel_desc boe_nv110wtm_n61 = { + .modes = boe_nv110wtm_n61_modes, + .num_modes = ARRAY_SIZE(boe_nv110wtm_n61_modes), + .bpc = 8, + .size = { + .width = 233, + .height = 155, + }, + .delay = { + .hpd_absent_delay = 200, + }, + .timing_constraints = { + .prepare_to_enable_ms = 80, + .unprepare_to_prepare_ms = 500, + }, + .bus_format = MEDIA_BUS_FMT_RGB888_1X24, + .bus_flags = DRM_BUS_FLAG_DATA_MSB_TO_LSB, + .connector_type = DRM_MODE_CONNECTOR_eDP, +}; + /* Also used for boe_nv133fhm_n62 */ static const struct drm_display_mode boe_nv133fhm_n61_modes = { .clock = 147840, @@ -4015,6 +4060,9 @@ static const struct of_device_id platform_of_match[] = { }, { .compatible = "boe,nv101wxmn51", .data = _nv101wxmn51, + }, { + .compatible = "boe,nv110wtm-n61", + .data = _nv110wtm_n61, }, { .compatible = "boe,nv133fhm-n61", .data = _nv133fhm_n61, -- 2.29.0.rc2.309.g374f81d7ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] dt-bindings: dt-bindings: display: simple: Add BOE NV110WTM-N61
Add yet another eDP panel. Signed-off-by: Douglas Anderson --- .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index edb53ab0d9eb..93e244c67e8a 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -76,6 +76,8 @@ properties: # BOE OPTOELECTRONICS TECHNOLOGY 10.1" WXGA TFT LCD panel - boe,nv101wxmn51 # BOE NV133FHM-N61 13.3" FHD (1920x1080) TFT LCD Panel + - boe,nv110wtm-n61 +# BOE NV110WTM-N61 11.0" 2160x1440 TFT LCD Panel - boe,nv133fhm-n61 # BOE NV133FHM-N62 13.3" FHD (1920x1080) TFT LCD Panel - boe,nv133fhm-n62 -- 2.29.0.rc2.309.g374f81d7ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Subject: [RFC] clang tooling cleanups
This rfc will describe An upcoming treewide cleanup. How clang tooling was used to programatically do the clean up. Solicit opinions on how to generally use clang tooling. The clang warning -Wextra-semi-stmt produces about 10k warnings. Reviewing these, a subset of semicolon after a switch looks safe to fix all the time. An example problem void foo(int a) { switch(a) { case 1: ... }; <--- extra semicolon } Treewide, there are about 100 problems in 50 files for x86_64 allyesconfig. These fixes will be the upcoming cleanup. clang already supports fixing this problem. Add to your command line clang -c -Wextra-semi-stmt -Xclang -fixit foo.c foo.c:8:3: warning: empty expression statement has no effect; remove unnecessary ';' to silence this warning [-Wextra-semi-stmt] }; ^ foo.c:8:3: note: FIX-IT applied suggested code changes 1 warning generated. The big problem is using this treewide is it will fix all 10k problems. 10k changes to analyze and upstream is not practical. Another problem is the generic fixer only removes the semicolon. So empty lines with some tabs need to be manually cleaned. What is needed is a more precise fixer. Enter clang-tidy. https://clang.llvm.org/extra/clang-tidy/ Already part of the static checker infrastructure, invoke on the clang build with make clang-tidy It is only a matter of coding up a specific checker for the cleanup. Upstream this is review is happening here https://reviews.llvm.org/D90180 The development of a checker/fixer is Start with a reproducer void foo (int a) { switch (a) {}; } Generate the abstract syntax tree (AST) clang -Xclang -ast-dump foo.c `-FunctionDecl |-ParmVarDecl `-CompoundStmt |-SwitchStmt | |-ImplicitCastExpr | | `-DeclRefExpr | `-CompoundStmt `-NullStmt Write a matcher to get you most of the way void SwitchSemiCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( compoundStmt(has(switchStmt().bind("switch"))).bind("comp"), this); } The 'bind' method is important, it allows a string to be associated with a node in the AST. In this case these are `-FunctionDecl |-ParmVarDecl `-CompoundStmt < comp |-SwitchStmt < switch | |-ImplicitCastExpr | | `-DeclRefExpr | `-CompoundStmt `-NullStmt When a match is made the 'check' method will be called. void SwitchSemiCheck::check(const MatchFinder::MatchResult ) { auto *C = Result.Nodes.getNodeAs("comp"); auto *S = Result.Nodes.getNodeAs("switch"); This is where the string in the bind calls are changed to nodes `-FunctionDecl |-ParmVarDecl `-CompoundStmt < comp, C |-SwitchStmt < switch, S | |-ImplicitCastExpr | | `-DeclRefExpr | `-CompoundStmt `-NullStmt <-- looking for N And then more logic to find the NullStmt auto Current = C->body_begin(); auto Next = Current; Next++; while (Next != C->body_end()) { if (*Current == S) { if (const auto *N = dyn_cast(*Next)) { When it is found, a warning is printed and a FixItHint is proposed. auto H = FixItHint::CreateReplacement( SourceRange(S->getBody()->getEndLoc(), N->getSemiLoc()), "}"); diag(N->getSemiLoc(), "unneeded semicolon") << H; This fixit replaces from the end of switch to the semicolon with a '}'. Because the end of the switch is '}' this has the effect of removing all the whitespace as well as the semicolon. Because of the checker's placement in clang-tidy existing linuxkernel checkers, all that was needed to fix the tree was to add a '-fix'to the build's clang-tidy call. I am looking for opinions on what we want to do specifically with cleanups and generally about other source-to-source programmatic changes to the code base. For cleanups, I think we need a new toplevel target clang-tidy-fix And an explicit list of fixers that have a very high (100%?) fix rate. Ideally a bot should make the changes, but a bot could also nag folks. Is there interest in a bot making the changes? Does one already exist? The general source-to-source is a bit blue sky. Ex/ could automagicly refactor api, outline similar cut-n-pasted functions etc. Anything on someone's wishlist you want to try out ? Signed-off-by: Tom Rix ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] parisc/sticore: Avoid hard-coding built-in font charcount
sti_select_fbfont() and sti_cook_fonts() are hard-coding the number of characters of our built-in fonts as 256. Recently, we included that information in our kernel font descriptor `struct font_desc`, so use `fbfont->charcount` instead of hard-coded values. This patch depends on patch "Fonts: Add charcount field to font_desc". Signed-off-by: Peilin Ye --- $ # Build-tested (Ubuntu 20.04) $ sudo apt-get install binutils-hppa64-linux-gnu gcc-7-hppa64-linux-gnu $ cp arch/parisc/configs/generic-64bit_defconfig .config $ make -j`nproc` ARCH=parisc CROSS_COMPILE=hppa64-linux-gnu- all drivers/video/console/sticore.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c index d1bb5915082b..f869b723494f 100644 --- a/drivers/video/console/sticore.c +++ b/drivers/video/console/sticore.c @@ -506,7 +506,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name) fbfont->width, fbfont->height, fbfont->name); bpc = ((fbfont->width+7)/8) * fbfont->height; - size = bpc * 256; + size = bpc * fbfont->charcount; size += sizeof(struct sti_rom_font); nf = kzalloc(size, STI_LOWMEM); @@ -514,7 +514,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name) return NULL; nf->first_char = 0; - nf->last_char = 255; + nf->last_char = fbfont->charcount - 1; nf->width = fbfont->width; nf->height = fbfont->height; nf->font_type = STI_FONT_HPROMAN8; @@ -525,7 +525,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name) dest = nf; dest += sizeof(struct sti_rom_font); - memcpy(dest, fbfont->data, bpc*256); + memcpy(dest, fbfont->data, bpc * fbfont->charcount); cooked_font = kzalloc(sizeof(*cooked_font), GFP_KERNEL); if (!cooked_font) { @@ -660,7 +660,7 @@ static int sti_cook_fonts(struct sti_cooked_rom *cooked_rom, void sti_font_convert_bytemode(struct sti_struct *sti, struct sti_cooked_font *f) { unsigned char *n, *p, *q; - int size = f->raw->bytes_per_char * 256 + sizeof(struct sti_rom_font); + int size = f->raw->bytes_per_char * (f->raw->last_char + 1) + sizeof(struct sti_rom_font); struct sti_rom_font *old_font; if (sti->wordmode) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] fbcon: Avoid hard-coding built-in font charcount
fbcon_startup() and fbcon_init() are hard-coding the number of characters of our built-in fonts as 256. Recently, we included that information in our kernel font descriptor `struct font_desc`, so use `font->charcount` instead of a hard-coded value. This patch depends on patch "Fonts: Add charcount field to font_desc". Signed-off-by: Peilin Ye --- drivers/video/fbdev/core/fbcon.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index cef437817b0d..e563847991b7 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -1004,7 +1004,7 @@ static const char *fbcon_startup(void) vc->vc_font.width = font->width; vc->vc_font.height = font->height; vc->vc_font.data = (void *)(p->fontdata = font->data); - vc->vc_font.charcount = 256; /* FIXME Need to support more fonts */ + vc->vc_font.charcount = font->charcount; } else { p->fontdata = vc->vc_font.data; } @@ -1083,8 +1083,7 @@ static void fbcon_init(struct vc_data *vc, int init) vc->vc_font.width = font->width; vc->vc_font.height = font->height; vc->vc_font.data = (void *)(p->fontdata = font->data); - vc->vc_font.charcount = 256; /* FIXME Need to - support more fonts */ + vc->vc_font.charcount = font->charcount; } } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] Fonts: Add charcount field to font_desc
Subsystems are assuming the number of characters of our built-in fonts. Include that information in our kernel font descriptor, `struct font_desc`. Signed-off-by: Peilin Ye --- include/linux/font.h | 1 + lib/fonts/font_10x18.c | 1 + lib/fonts/font_6x10.c | 1 + lib/fonts/font_6x11.c | 1 + lib/fonts/font_6x8.c | 1 + lib/fonts/font_7x14.c | 1 + lib/fonts/font_8x16.c | 1 + lib/fonts/font_8x8.c | 1 + lib/fonts/font_acorn_8x8.c | 1 + lib/fonts/font_mini_4x6.c | 1 + lib/fonts/font_pearl_8x8.c | 1 + lib/fonts/font_sun12x22.c | 1 + lib/fonts/font_sun8x16.c | 1 + lib/fonts/font_ter16x32.c | 1 + 14 files changed, 14 insertions(+) diff --git a/include/linux/font.h b/include/linux/font.h index 4f50d736ea72..abf1442ce719 100644 --- a/include/linux/font.h +++ b/include/linux/font.h @@ -17,6 +17,7 @@ struct font_desc { int idx; const char *name; unsigned int width, height; +unsigned int charcount; const void *data; int pref; }; diff --git a/lib/fonts/font_10x18.c b/lib/fonts/font_10x18.c index 0e2deac97da0..4096c6562494 100644 --- a/lib/fonts/font_10x18.c +++ b/lib/fonts/font_10x18.c @@ -5137,6 +5137,7 @@ const struct font_desc font_10x18 = { .name = "10x18", .width = 10, .height = 18, + .charcount = 256, .data = fontdata_10x18.data, #ifdef __sparc__ .pref = 5, diff --git a/lib/fonts/font_6x10.c b/lib/fonts/font_6x10.c index 87da8acd07db..32786674cf65 100644 --- a/lib/fonts/font_6x10.c +++ b/lib/fonts/font_6x10.c @@ -3083,6 +3083,7 @@ const struct font_desc font_6x10 = { .name = "6x10", .width = 6, .height = 10, + .charcount = 256, .data = fontdata_6x10.data, .pref = 0, }; diff --git a/lib/fonts/font_6x11.c b/lib/fonts/font_6x11.c index 5e975dfa10a5..81e4a3aed44a 100644 --- a/lib/fonts/font_6x11.c +++ b/lib/fonts/font_6x11.c @@ -3346,6 +3346,7 @@ const struct font_desc font_vga_6x11 = { .name = "ProFont6x11", .width = 6, .height = 11, + .charcount = 256, .data = fontdata_6x11.data, /* Try avoiding this font if possible unless on MAC */ .pref = -2000, diff --git a/lib/fonts/font_6x8.c b/lib/fonts/font_6x8.c index 700039a9ceae..5618ae7ef9fa 100644 --- a/lib/fonts/font_6x8.c +++ b/lib/fonts/font_6x8.c @@ -2571,6 +2571,7 @@ const struct font_desc font_6x8 = { .name = "6x8", .width = 6, .height = 8, + .charcount = 256, .data = fontdata_6x8.data, .pref = 0, }; diff --git a/lib/fonts/font_7x14.c b/lib/fonts/font_7x14.c index 86d298f38505..7708e73d491f 100644 --- a/lib/fonts/font_7x14.c +++ b/lib/fonts/font_7x14.c @@ -4113,6 +4113,7 @@ const struct font_desc font_7x14 = { .name = "7x14", .width = 7, .height = 14, + .charcount = 256, .data = fontdata_7x14.data, .pref = 0, }; diff --git a/lib/fonts/font_8x16.c b/lib/fonts/font_8x16.c index 37cedd36ca5e..74125d3570cf 100644 --- a/lib/fonts/font_8x16.c +++ b/lib/fonts/font_8x16.c @@ -4627,6 +4627,7 @@ const struct font_desc font_vga_8x16 = { .name = "VGA8x16", .width = 8, .height = 16, + .charcount = 256, .data = fontdata_8x16.data, .pref = 0, }; diff --git a/lib/fonts/font_8x8.c b/lib/fonts/font_8x8.c index 8ab695538395..96da4bb31ae4 100644 --- a/lib/fonts/font_8x8.c +++ b/lib/fonts/font_8x8.c @@ -2578,6 +2578,7 @@ const struct font_desc font_vga_8x8 = { .name = "VGA8x8", .width = 8, .height = 8, + .charcount = 256, .data = fontdata_8x8.data, .pref = 0, }; diff --git a/lib/fonts/font_acorn_8x8.c b/lib/fonts/font_acorn_8x8.c index 069b3e80c434..ba74053fec7b 100644 --- a/lib/fonts/font_acorn_8x8.c +++ b/lib/fonts/font_acorn_8x8.c @@ -270,6 +270,7 @@ const struct font_desc font_acorn_8x8 = { .name = "Acorn8x8", .width = 8, .height = 8, + .charcount = 256, .data = acorndata_8x8.data, #ifdef CONFIG_ARCH_ACORN .pref = 20, diff --git a/lib/fonts/font_mini_4x6.c b/lib/fonts/font_mini_4x6.c index 1449876c6a27..637708e8c67e 100644 --- a/lib/fonts/font_mini_4x6.c +++ b/lib/fonts/font_mini_4x6.c @@ -2152,6 +2152,7 @@ const struct font_desc font_mini_4x6 = { .name = "MINI4x6", .width = 4, .height = 6, + .charcount = 256, .data = fontdata_mini_4x6.data, .pref = 3, }; diff --git a/lib/fonts/font_pearl_8x8.c b/lib/fonts/font_pearl_8x8.c index 32d65551e7ed..06cde43c7bd2 100644 --- a/lib/fonts/font_pearl_8x8.c +++ b/lib/fonts/font_pearl_8x8.c @@ -2582,6 +2582,7 @@ const struct font_desc font_pearl_8x8 = { .name = "PEARL8x8", .width = 8, .height = 8, + .charcount = 256, .data = fontdata_pearl8x8.data, .pref = 2, }; diff --git a/lib/fonts/font_sun12x22.c
[PATCH 2/5] Fonts: Make font size unsigned in font_desc
It is improper to define `width` and `height` as signed in `struct font_desc`. Make them unsigned. Also, change the corresponding printk() format identifiers from `%d` to `%u`, in sti_select_fbfont(). Signed-off-by: Peilin Ye --- Build-tested. drivers/video/console/sticore.c | 2 +- include/linux/font.h| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/console/sticore.c b/drivers/video/console/sticore.c index 6a26a364f9bd..d1bb5915082b 100644 --- a/drivers/video/console/sticore.c +++ b/drivers/video/console/sticore.c @@ -502,7 +502,7 @@ sti_select_fbfont(struct sti_cooked_rom *cooked_rom, const char *fbfont_name) if (!fbfont) return NULL; - pr_info("STI selected %dx%d framebuffer font %s for sticon\n", + pr_info("STI selected %ux%u framebuffer font %s for sticon\n", fbfont->width, fbfont->height, fbfont->name); bpc = ((fbfont->width+7)/8) * fbfont->height; diff --git a/include/linux/font.h b/include/linux/font.h index b5b312c19e46..4f50d736ea72 100644 --- a/include/linux/font.h +++ b/include/linux/font.h @@ -16,7 +16,7 @@ struct font_desc { int idx; const char *name; -int width, height; +unsigned int width, height; const void *data; int pref; }; -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] fbdev/atafb: Remove unused extern variables
Remove 6 unused extern variables to reduce confusion. It is worth mentioning that lib/fonts/font_8x8.c and lib/fonts/font_8x16.c also declare `fontdata_8x8` and `fontdata_8x16` respectively, and this file has nothing to do with them. Signed-off-by: Peilin Ye --- $ # Build-tested (Ubuntu 20.04) $ sudo apt install gcc-m68k-linux-gnu $ cp arch/m68k/configs/atari_defconfig .config $ make ARCH=m68k menuconfig $ make ARCH=m68k CROSS_COMPILE=m68k-linux-gnu- -j`nproc` all drivers/video/fbdev/atafb.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c index f253daa05d9d..e3812a8ff55a 100644 --- a/drivers/video/fbdev/atafb.c +++ b/drivers/video/fbdev/atafb.c @@ -240,14 +240,6 @@ static int *MV300_reg = MV300_reg_8bit; static int inverse; -extern int fontheight_8x8; -extern int fontwidth_8x8; -extern unsigned char fontdata_8x8[]; - -extern int fontheight_8x16; -extern int fontwidth_8x16; -extern unsigned char fontdata_8x16[]; - /* * struct fb_ops { * * open/release and usage marking -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/5] Preparation work for using font_desc in vc_data
Hi Daniel, Hi Greg, Hi all, We are planning to use `font_desc` instead of `console_font` in `vc_data`, and this is just some prep work for it. It doesn't do much, but at least it removes two "FIXME"s in fbcon.c :) Peilin Ye (5): [1/5] fbdev/atafb: Remove unused extern variables Searching for `fontdata` gave me this in fbdev/atafb.c: extern unsigned char fontdata_8x8[]; extern unsigned char fontdata_8x16[]; ...which freaked me out, since in 6735b4632def ("Fonts: Support FONT_EXTRA_WORDS macros for built-in fonts") I changed them from char arrays to structures, in lib/fonts/. Fortunately it turns out these extern variables have nothing to do with lib/fonts/, and are not being used anywhere, so remove them for less confusion. m68k cross-compiled. [2/5] Fonts: Make font size unsigned in font_desc Our goal is to use `font_desc` "everywhere" in the kernel, and signed `width` and `height` is inappropriate. Also, change some printk() format identifiers in console/sticore.c from `%d` to `%u`. parisc cross-compiled. [3/5] Fonts: Add charcount field to font_desc Add `unsigned int charcount` to `font_desc`, and update each of our 13 built-in fonts. [4/5] fbcon: Avoid hard-coding built-in font charcount [5/5] parisc/sticore: Avoid hard-coding built-in font charcount Everyone (tty, fbcon, sticore, etc.) is assuming that all built-in fonts have 256 characters, and is using hard-coded 256 or 255 everywhere. These two patches removes some of them. [5/5] is parisc cross-compiled. Now is a good time to review all find_font() and get_default_font() callers: drivers/media/pci/solo6x10/solo6x10-enc.c 133 const struct font_desc *vga = find_font("VGA8x16"); drivers/media/test-drivers/vimc/vimc-core.c268 const struct font_desc *font = find_font("VGA8x16"); drivers/media/test-drivers/vivid/vivid-core.c 1928 const struct font_desc *font = find_font("VGA8x16"); drivers/usb/misc/sisusbvga/sisusb.c 2285 myfont = find_font("VGA8x16"); * These 4 only care about font VGA8x16, so let them be for now; drivers/video/console/sticore.c499 fbfont = find_font(fbfont_name); drivers/video/console/sticore.c501 fbfont = get_default_font(1024,768, ~(u32)0, ~(u32)0); * Uses 255 and 256, (hopefully) cleaned up by [5/5]; drivers/video/fbdev/core/fbcon.c 999 if (!fontname[0] || !(font = find_font(fontname))) drivers/video/fbdev/core/fbcon.c 1000 font = get_default_font(info->var.xres, drivers/video/fbdev/core/fbcon.c 1078 if (!fontname[0] || !(font = find_font(fontname))) drivers/video/fbdev/core/fbcon.c 1079 font = get_default_font(info->var.xres, * Use 256, cleaned up by [4/5]; drivers/video/fbdev/core/fbcon.c 2548 else if (!(f = find_font(name))) drivers/video/fbdev/core/fbcon.c 2546 f = get_default_font(info->var.xres, info->var.yres, * Uses 256 but no easy fix. I'll clean this up after making fbcon_do_set_font() pass a `font_desc` as parameter; drivers/firmware/efi/earlycon.c 234 font = get_default_font(xres, yres, -1, -1); * Does not care about charcount. Thank you! Peilin Ye drivers/video/console/sticore.c | 10 +- drivers/video/fbdev/atafb.c | 8 drivers/video/fbdev/core/fbcon.c | 5 ++--- include/linux/font.h | 3 ++- lib/fonts/font_10x18.c | 1 + lib/fonts/font_6x10.c| 1 + lib/fonts/font_6x11.c| 1 + lib/fonts/font_6x8.c | 1 + lib/fonts/font_7x14.c| 1 + lib/fonts/font_8x16.c| 1 + lib/fonts/font_8x8.c | 1 + lib/fonts/font_acorn_8x8.c | 1 + lib/fonts/font_mini_4x6.c| 1 + lib/fonts/font_pearl_8x8.c | 1 + lib/fonts/font_sun12x22.c| 1 + lib/fonts/font_sun8x16.c | 1 + lib/fonts/font_ter16x32.c| 1 + 17 files changed, 22 insertions(+), 17 deletions(-) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context
On Tue, Oct 27, 2020 at 11:15:58AM +0100, Maxime Ripard wrote: > When running the trigger hook, ALSA by default will take a spinlock, and > thus will run the trigger hook in atomic context. Reviewed-by: Mark Brown signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/a6xx: Add support for using system cache on MMU500 based targets
On 2020-10-26 18:54, Jordan Crouse wrote: This is an extension to the series [1] to enable the System Cache (LLC) for Adreno a6xx targets. GPU targets with an MMU-500 attached have a slightly different process for enabling system cache. Use the compatible string on the IOMMU phandle to see if an MMU-500 is attached and modify the programming sequence accordingly. [1] https://patchwork.freedesktop.org/series/83037/ Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 46 +-- drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index 95c98c642876..b7737732fbb6 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -1042,6 +1042,8 @@ static void a6xx_llc_deactivate(struct a6xx_gpu *a6xx_gpu) static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu) { + struct adreno_gpu *adreno_gpu = _gpu->base; + struct msm_gpu *gpu = _gpu->base; u32 cntl1_regval = 0; if (IS_ERR(a6xx_gpu->llc_mmio)) @@ -1055,11 +1057,17 @@ static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu) (gpu_scid << 15) | (gpu_scid << 20); } + /* +* For targets with a MMU500, activate the slice but don't program the +* register. The XBL will take care of that. +*/ if (!llcc_slice_activate(a6xx_gpu->htw_llc_slice)) { - u32 gpuhtw_scid = llcc_get_slice_id(a6xx_gpu->htw_llc_slice); + if (!a6xx_gpu->have_mmu500) { + u32 gpuhtw_scid = llcc_get_slice_id(a6xx_gpu->htw_llc_slice); - gpuhtw_scid &= 0x1f; - cntl1_regval |= FIELD_PREP(GENMASK(29, 25), gpuhtw_scid); + gpuhtw_scid &= 0x1f; + cntl1_regval |= FIELD_PREP(GENMASK(29, 25), gpuhtw_scid); + } } if (cntl1_regval) { @@ -1067,13 +1075,20 @@ static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu) * Program the slice IDs for the various GPU blocks and GPU MMU * pagetables */ - a6xx_llc_write(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_1, cntl1_regval); - - /* -* Program cacheability overrides to not allocate cache lines on -* a write miss -*/ - a6xx_llc_rmw(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_0, 0xF, 0x03); + if (a6xx_gpu->have_mmu500) + gpu_rmw(gpu, REG_A6XX_GBIF_SCACHE_CNTL1, GENMASK(24, 0), + cntl1_regval); + else { + a6xx_llc_write(a6xx_gpu, + REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_1, cntl1_regval); + + /* +* Program cacheability overrides to not allocate cache +* lines on a write miss +*/ + a6xx_llc_rmw(a6xx_gpu, + REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_0, 0xF, 0x03); + } } } @@ -1086,10 +1101,21 @@ static void a6xx_llc_slices_destroy(struct a6xx_gpu *a6xx_gpu) static void a6xx_llc_slices_init(struct platform_device *pdev, struct a6xx_gpu *a6xx_gpu) { + struct device_node *phandle; + a6xx_gpu->llc_mmio = msm_ioremap(pdev, "cx_mem", "gpu_cx"); if (IS_ERR(a6xx_gpu->llc_mmio)) return; + /* +* There is a different programming path for targets with an mmu500 +* attached, so detect if that is the case +*/ + phandle = of_parse_phandle(pdev->dev.of_node, "iommus", 0); + a6xx_gpu->have_mmu500 = (phandle && + of_device_is_compatible(phandle, "arm,mmu500")); Note that this should never match, since the compatible string defined by the binding is "arm,mmu-500" ;) Robin. + of_node_put(phandle); + a6xx_gpu->llc_slice = llcc_slice_getd(LLCC_GPU); a6xx_gpu->htw_llc_slice = llcc_slice_getd(LLCC_GPUHTW); diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h index 9e6079af679c..e793d329e77b 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h @@ -32,6 +32,7 @@ struct a6xx_gpu { void __iomem *llc_mmio; void *llc_slice; void *htw_llc_slice; + bool have_mmu500; }; #define to_a6xx_gpu(x) container_of(x, struct a6xx_gpu, base) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: bridge: cdns: Kconfig: Switch over dependency to ARCH_K3
On 26/10/2020 18:54, Nishanth Menon wrote: > With the integration of chip-id detection scheme in kernel[1], there > is no specific need to maintain multitudes of SoC specific config > options, discussed as per [2], we have deprecated the usage in other > places for v5.10-rc1. Fix the missing user so that we can clean up the > configs in v5.11. > > [1] drivers/soc/ti/k3-socinfo.c commit 907a2b7e2fc7 ("soc: ti: add k3 > platforms chipid module driver") > [2] > https://lore.kernel.org/linux-arm-kernel/20200908112534.t5bgrjf7y3a6l2ss@akan/ > > Fixes: afba7e6c5fc1 ("rm: bridge: cdns-mhdp8546: Add TI J721E wrapper") > Cc: Swapnil Jakhade > Cc: Tomi Valkeinen > Cc: Laurent Pinchart > Cc: Yuti Amonkar > Cc: Jyri Sarha > Signed-off-by: Nishanth Menon > --- > drivers/gpu/drm/bridge/cadence/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig > b/drivers/gpu/drm/bridge/cadence/Kconfig > index 511d67b16d14..ef8c230e0f62 100644 > --- a/drivers/gpu/drm/bridge/cadence/Kconfig > +++ b/drivers/gpu/drm/bridge/cadence/Kconfig > @@ -13,7 +13,7 @@ config DRM_CDNS_MHDP8546 > if DRM_CDNS_MHDP8546 > > config DRM_CDNS_MHDP8546_J721E > - depends on ARCH_K3_J721E_SOC || COMPILE_TEST > + depends on ARCH_K3 || COMPILE_TEST > bool "J721E Cadence DPI/DP wrapper support" > default y > help > Reviewed-by: Tomi Valkeinen Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vboxvideo: Unmap iomem on probe-failure and remove
On Tue, Oct 27, 2020 at 2:52 PM Hans de Goede wrote: > > Add missing pci_iounmap() calls to properly unmap the memory on > probe-failure and remove. > > Reported-by: kernel test robot > Reported-by: Dan Carpenter > Signed-off-by: Hans de Goede I think switching over to devm would be really nice. And for pci all you need to do is use pcim_enable_device and delete all the cleanup code, and it's all done. Hand rolling device cleanup code really isn't a great idea and way too error-prone. Plus you're using lots of devm_ already. -Daniel > --- > drivers/gpu/drm/vboxvideo/vbox_main.c | 23 --- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/vboxvideo/vbox_main.c > b/drivers/gpu/drm/vboxvideo/vbox_main.c > index d68d9bad7674..2eeb1d3be54a 100644 > --- a/drivers/gpu/drm/vboxvideo/vbox_main.c > +++ b/drivers/gpu/drm/vboxvideo/vbox_main.c > @@ -71,6 +71,8 @@ static void vbox_accel_fini(struct vbox_private *vbox) > > for (i = 0; i < vbox->num_crtcs; ++i) > vbva_disable(>vbva_info[i], vbox->guest_pool, i); > + > + pci_iounmap(vbox->ddev.pdev, vbox->vbva_buffers); > } > > /* Do we support the 4.3 plus mode hint reporting interface? */ > @@ -124,19 +126,19 @@ int vbox_hw_init(struct vbox_private *vbox) > vbox->guest_pool = devm_gen_pool_create(vbox->ddev.dev, 4, -1, > "vboxvideo-accel"); > if (!vbox->guest_pool) > - return -ENOMEM; > + goto out_unmap_guest_heap; > > ret = gen_pool_add_virt(vbox->guest_pool, > (unsigned long)vbox->guest_heap, > GUEST_HEAP_OFFSET(vbox), > GUEST_HEAP_USABLE_SIZE, -1); > if (ret) > - return ret; > + goto out_unmap_guest_heap; > > ret = hgsmi_test_query_conf(vbox->guest_pool); > if (ret) { > DRM_ERROR("vboxvideo: hgsmi_test_query_conf failed\n"); > - return ret; > + goto out_unmap_guest_heap; > } > > /* Reduce available VRAM size to reflect the guest heap. */ > @@ -148,23 +150,30 @@ int vbox_hw_init(struct vbox_private *vbox) > > if (!have_hgsmi_mode_hints(vbox)) { > ret = -ENOTSUPP; > - return ret; > + goto out_unmap_guest_heap; > } > > vbox->last_mode_hints = devm_kcalloc(vbox->ddev.dev, vbox->num_crtcs, > sizeof(struct vbva_modehint), > GFP_KERNEL); > - if (!vbox->last_mode_hints) > - return -ENOMEM; > + if (!vbox->last_mode_hints) { > + ret = -ENOMEM; > + goto out_unmap_guest_heap; > + } > > ret = vbox_accel_init(vbox); > if (ret) > - return ret; > + goto out_unmap_guest_heap; > > return 0; > + > +out_unmap_guest_heap: > + pci_iounmap(vbox->ddev.pdev, vbox->guest_pool); > + return ret; > } > > void vbox_hw_fini(struct vbox_private *vbox) > { > vbox_accel_fini(vbox); > + pci_iounmap(vbox->ddev.pdev, vbox->guest_pool); > } > -- > 2.28.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] drm/radeon: Add new callback that exposes vddc
Applied. Thanks! Alex On Tue, Oct 27, 2020 at 7:53 AM Sandeep wrote: > > On Mon, 26 Oct 2020 at 23:53, Alex Deucher wrote: > > > > I don't see them on the mailing list. Are you sure they went out? > > > > Alex > > The original email in this chain is the v2, sent in the correct form. > > > > > On Sat, Oct 24, 2020 at 1:47 PM Sandeep wrote: > > > > > > Hello, > > > > > > I've resent the patches in the correct format. Please review. > > > > > > - Sandeep > > > > > > On Fri, 9 Oct 2020 at 13:14, Sandeep Raghuraman > > > wrote: > > > > > > > > This patch adds a callback for reporting vddc, to the dpm field of the > > > > radeon_asic structure. > > > > > > > > Signed-off-by: Sandeep Raghuraman > > > > > > > > --- > > > > drivers/gpu/drm/radeon/radeon.h | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/gpu/drm/radeon/radeon.h > > > > b/drivers/gpu/drm/radeon/radeon.h > > > > index cc4f58d16589..85a1cafdf303 100644 > > > > --- a/drivers/gpu/drm/radeon/radeon.h > > > > +++ b/drivers/gpu/drm/radeon/radeon.h > > > > @@ -1992,6 +1992,7 @@ struct radeon_asic { > > > > int (*get_fan_speed_percent)(struct radeon_device > > > > *rdev, u32 *speed); > > > > u32 (*get_current_sclk)(struct radeon_device *rdev); > > > > u32 (*get_current_mclk)(struct radeon_device *rdev); > > > > + u16 (*get_current_vddc)(struct radeon_device *rdev); > > > > } dpm; > > > > /* pageflipping */ > > > > struct { > > > > -- > > > ___ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/8] vhost: vringh: use krealloc_array()
On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > Use the helper that checks for overflows internally instead of manually > calculating the size of the new array. > > Signed-off-by: Bartosz Golaszewski No problem with the patch, it does introduce some symmetry in the code. Acked-by: Michael S. Tsirkin > --- > drivers/vhost/vringh.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c > index 8bd8b403f087..08a0e1c842df 100644 > --- a/drivers/vhost/vringh.c > +++ b/drivers/vhost/vringh.c > @@ -198,7 +198,8 @@ static int resize_iovec(struct vringh_kiov *iov, gfp_t > gfp) > > flag = (iov->max_num & VRINGH_IOV_ALLOCATED); > if (flag) > - new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp); > + new = krealloc_array(iov->iov, new_num, > + sizeof(struct iovec), gfp); > else { > new = kmalloc_array(new_num, sizeof(struct iovec), gfp); > if (new) { > -- > 2.29.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/imx: tve remove extraneous type qualifier
On Mon, 2020-10-26 at 20:41 +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > clang warns about functions returning a 'const int' result: > > drivers/gpu/drm/imx/imx-tve.c:487:8: warning: type qualifiers ignored on > function return type [-Wignored-qualifiers] > > Remove the extraneous 'const' qualifier here. I would guess that the > function was intended to be marked __attribute__((const)) instead, > but that would also be wrong since it call other functions without > that attribute. > > Fixes: fcbc51e54d2a ("staging: drm/imx: Add support for Television Encoder > (TVEv2)") > Signed-off-by: Arnd Bergmann Thank you, applied to imx-drm/next with Nick's R-b. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/a6xx: Add support for using system cache on MMU500 based targets
On Tue, Oct 27, 2020 at 12:38:02PM +0530, Sai Prakash Ranjan wrote: > On 2020-10-27 00:24, Jordan Crouse wrote: > >This is an extension to the series [1] to enable the System Cache (LLC) > >for > >Adreno a6xx targets. > > > >GPU targets with an MMU-500 attached have a slightly different process for > >enabling system cache. Use the compatible string on the IOMMU phandle > >to see if an MMU-500 is attached and modify the programming sequence > >accordingly. > > > >[1] https://patchwork.freedesktop.org/series/83037/ > > > >Signed-off-by: Jordan Crouse > >--- > > > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 46 +-- > > drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + > > 2 files changed, 37 insertions(+), 10 deletions(-) > > > >diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >index 95c98c642876..b7737732fbb6 100644 > >--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >@@ -1042,6 +1042,8 @@ static void a6xx_llc_deactivate(struct a6xx_gpu > >*a6xx_gpu) > > > > static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu) > > { > >+struct adreno_gpu *adreno_gpu = _gpu->base; > >+struct msm_gpu *gpu = _gpu->base; > > u32 cntl1_regval = 0; > > > > if (IS_ERR(a6xx_gpu->llc_mmio)) > >@@ -1055,11 +1057,17 @@ static void a6xx_llc_activate(struct a6xx_gpu > >*a6xx_gpu) > >(gpu_scid << 15) | (gpu_scid << 20); > > } > > > >+/* > >+ * For targets with a MMU500, activate the slice but don't program the > >+ * register. The XBL will take care of that. > >+ */ > > if (!llcc_slice_activate(a6xx_gpu->htw_llc_slice)) { > >-u32 gpuhtw_scid = llcc_get_slice_id(a6xx_gpu->htw_llc_slice); > >+if (!a6xx_gpu->have_mmu500) { > >+u32 gpuhtw_scid = > >llcc_get_slice_id(a6xx_gpu->htw_llc_slice); > > > >-gpuhtw_scid &= 0x1f; > >-cntl1_regval |= FIELD_PREP(GENMASK(29, 25), gpuhtw_scid); > >+gpuhtw_scid &= 0x1f; > >+cntl1_regval |= FIELD_PREP(GENMASK(29, 25), > >gpuhtw_scid); > >+} > > } > > > > if (cntl1_regval) { > >@@ -1067,13 +1075,20 @@ static void a6xx_llc_activate(struct a6xx_gpu > >*a6xx_gpu) > > * Program the slice IDs for the various GPU blocks and GPU MMU > > * pagetables > > */ > >-a6xx_llc_write(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_1, > >cntl1_regval); > >- > >-/* > >- * Program cacheability overrides to not allocate cache lines on > >- * a write miss > >- */ > >-a6xx_llc_rmw(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_0, > >0xF, > >0x03); > >+if (a6xx_gpu->have_mmu500) > >+gpu_rmw(gpu, REG_A6XX_GBIF_SCACHE_CNTL1, GENMASK(24, 0), > >+cntl1_regval); > >+else { > >+a6xx_llc_write(a6xx_gpu, > >+REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_1, > >cntl1_regval); > >+ > >+/* > >+ * Program cacheability overrides to not allocate cache > >+ * lines on a write miss > >+ */ > >+a6xx_llc_rmw(a6xx_gpu, > >+REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_0, 0xF, > >0x03); > >+} > > } > > } > > > >@@ -1086,10 +1101,21 @@ static void a6xx_llc_slices_destroy(struct > >a6xx_gpu *a6xx_gpu) > > static void a6xx_llc_slices_init(struct platform_device *pdev, > > struct a6xx_gpu *a6xx_gpu) > > { > >+struct device_node *phandle; > >+ > > a6xx_gpu->llc_mmio = msm_ioremap(pdev, "cx_mem", "gpu_cx"); > > if (IS_ERR(a6xx_gpu->llc_mmio)) > > return; > > > >+/* > >+ * There is a different programming path for targets with an mmu500 > >+ * attached, so detect if that is the case > >+ */ > >+phandle = of_parse_phandle(pdev->dev.of_node, "iommus", 0); > >+a6xx_gpu->have_mmu500 = (phandle && > >+of_device_is_compatible(phandle, "arm,mmu500")); > >+of_node_put(phandle); > >+ > > a6xx_gpu->llc_slice = llcc_slice_getd(LLCC_GPU); > > a6xx_gpu->htw_llc_slice = llcc_slice_getd(LLCC_GPUHTW); > > > >diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >index 9e6079af679c..e793d329e77b 100644 > >--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >@@ -32,6 +32,7 @@ struct a6xx_gpu { > > void __iomem *llc_mmio; > > void *llc_slice; > > void *htw_llc_slice; > >+bool have_mmu500; > > }; > > > > #define to_a6xx_gpu(x) container_of(x, struct a6xx_gpu, base) > > Thanks Jordan for the patch. If it makes your life or Rob's life easier, please feel free to squash them. Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code
Re: [PATCH 2/4] disp/msm/dpu: add support to dump dpu registers
Hi Abhinav, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-exynos/exynos-drm-next] [also build test WARNING on drm-intel/for-linux-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.10-rc1 next-20201027] [cannot apply to drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507 base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next config: arm64-randconfig-s032-20201026 (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.3-56-gc09e8239-dirty # https://github.com/0day-ci/linux/commit/a7e6907c303a46ea8422fc3c414c22fdfb45d49f git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Abhinav-Kumar/Add-devcoredump-support-for-DPU/20201022-130507 git checkout a7e6907c303a46ea8422fc3c414c22fdfb45d49f # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot "sparse warnings: (new ones prefixed by >>)" >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:99:50: sparse: sparse: >> incorrect type in argument 1 (different address spaces) @@ expected void >> const volatile [noderef] __iomem *addr @@ got char * @@ >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:99:50: sparse: expected >> void const volatile [noderef] __iomem *addr >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:99:50: sparse: got char * drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:100:56: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got char * @@ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:100:56: sparse: expected void const volatile [noderef] __iomem *addr drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:100:56: sparse: got char * drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:102:56: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got char * @@ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:102:56: sparse: expected void const volatile [noderef] __iomem *addr drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:102:56: sparse: got char * drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:104:56: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got char * @@ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:104:56: sparse: expected void const volatile [noderef] __iomem *addr drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:104:56: sparse: got char * >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:211:30: sparse: sparse: >> incorrect type in assignment (different address spaces) @@ expected char >> *addr @@ got void [noderef] __iomem * @@ >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:211:30: sparse: expected >> char *addr >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:211:30: sparse: got void >> [noderef] __iomem * >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:220:44: sparse: sparse: >> incorrect type in argument 4 (different address spaces) @@ expected char >> *base_addr @@ got void [noderef] __iomem *base @@ >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:220:44: sparse: expected >> char *base_addr >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:220:44: sparse: got void >> [noderef] __iomem *base >> drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:229:22: sparse: sparse: >> incorrect type in assignment (different address spaces) @@ expected char >> *addr @@ got void [noderef] __iomem *base @@ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:229:22: sparse: expected char *addr drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:229:22: sparse: got void [noderef] __iomem *base drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:232:36: sparse: sparse: incorrect type in argument 4 (different address spaces) @@ expected char *base_addr @@ got void [noderef] __iomem *base @@ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c:232:36: sparse: expected char *base_addr
Re: [PATCH v6 33/52] memory: tegra20: Support interconnect framework
On Mon, Oct 26, 2020 at 01:17:16AM +0300, Dmitry Osipenko wrote: > Now Internal and External Memory Controllers are memory interconnection > providers. This allows us to use interconnect API for tuning of memory > configuration. EMC driver now supports OPPs and DVFS. > > Signed-off-by: Dmitry Osipenko > --- > drivers/memory/tegra/Kconfig | 3 +- > drivers/memory/tegra/mc.h | 12 ++ > drivers/memory/tegra/tegra20-emc.c | 176 + > drivers/memory/tegra/tegra20.c | 34 ++ > 4 files changed, 224 insertions(+), 1 deletion(-) > > diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig > index ff426747cd7d..ac3dfe155505 100644 > --- a/drivers/memory/tegra/Kconfig > +++ b/drivers/memory/tegra/Kconfig > @@ -11,7 +11,8 @@ config TEGRA_MC > config TEGRA20_EMC > tristate "NVIDIA Tegra20 External Memory Controller driver" > default y > - depends on ARCH_TEGRA_2x_SOC > + depends on TEGRA_MC && ARCH_TEGRA_2x_SOC > + select PM_OPP > help > This driver is for the External Memory Controller (EMC) found on > Tegra20 chips. The EMC controls the external DRAM on the board. > diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h > index abeb6a2cc36a..531fb4fb7b17 100644 > --- a/drivers/memory/tegra/mc.h > +++ b/drivers/memory/tegra/mc.h > @@ -78,6 +78,18 @@ > > #define MC_TIMING_UPDATE BIT(0) > > +static inline u32 tegra_mc_scale_percents(u64 val, unsigned int percents) > +{ > + val = val * percents; > + do_div(val, 100); > + > + /* > + * High freq + high boosting percent + large polling interval are > + * resulting in integer overflow when watermarks are calculated. > + */ > + return min_t(u64, val, U32_MAX); > +} > + > static inline u32 mc_readl(struct tegra_mc *mc, unsigned long offset) > { > return readl_relaxed(mc->regs + offset); > diff --git a/drivers/memory/tegra/tegra20-emc.c > b/drivers/memory/tegra/tegra20-emc.c > index 34085e26dced..69ccb3fe5b0b 100644 > --- a/drivers/memory/tegra/tegra20-emc.c > +++ b/drivers/memory/tegra/tegra20-emc.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -16,11 +17,15 @@ > #include > #include > #include > +#include > +#include > #include > #include > > #include > > +#include "mc.h" > + > #define EMC_INTSTATUS0x000 > #define EMC_INTMASK 0x004 > #define EMC_DBG 0x008 > @@ -144,6 +149,9 @@ struct emc_timing { > > struct tegra_emc { > struct device *dev; > + struct tegra_mc *mc; > + struct opp_table *opp_table; > + struct icc_provider provider; > struct notifier_block clk_nb; > struct clk *clk; > void __iomem *regs; > @@ -658,6 +666,166 @@ static void tegra_emc_debugfs_init(struct tegra_emc > *emc) > emc, _emc_debug_max_rate_fops); > } > > +static inline struct tegra_emc * > +to_tegra_emc_provider(struct icc_provider *provider) > +{ > + return container_of(provider, struct tegra_emc, provider); > +} > + > +static struct icc_node_data * > +emc_of_icc_xlate_extended(struct of_phandle_args *spec, void *data) > +{ > + struct icc_provider *provider = data; > + struct icc_node_data *ndata; > + struct icc_node *node; > + > + /* External Memory is the only possible ICC route */ > + list_for_each_entry(node, >nodes, node_list) { > + if (node->id != TEGRA_ICC_EMEM) > + continue; > + > + ndata = kzalloc(sizeof(*ndata), GFP_KERNEL); > + if (!ndata) > + return ERR_PTR(-ENOMEM); > + > + /* > + * SRC and DST nodes should have matching TAG in order to have > + * it set by default for a requested path. > + */ > + ndata->tag = TEGRA_MC_ICC_TAG_ISO; > + ndata->node = node; > + > + return ndata; > + } > + > + return ERR_PTR(-EINVAL); > +} > + > +static int emc_icc_set(struct icc_node *src, struct icc_node *dst) > +{ > + struct tegra_emc *emc = to_tegra_emc_provider(dst->provider); > + unsigned long long peak_bw = icc_units_to_bps(dst->peak_bw); > + unsigned long long avg_bw = icc_units_to_bps(dst->avg_bw); > + unsigned long long rate = max(avg_bw, peak_bw); > + unsigned int dram_data_bus_width_bytes = 4; Perhaps use something shorter for this variable (like dram_bus_width)? Also, since it's never modified, perhaps make it const? Or a #define? > + long rounded_rate; > + int err; > + > + /* > + * Tegra20 EMC runs on x2 clock rate of SDRAM bus because DDR data > + * is sampled on both clock edges. This means that EMC clock rate > + * equals to the peak data rate. > + */ > + do_div(rate, dram_data_bus_width_bytes);
Re: [PATCH v6 31/52] memory: tegra20-emc: Use devm_platform_ioremap_resource()
On Tue, 27 Oct 2020 at 14:50, Thierry Reding wrote: > > On Mon, Oct 26, 2020 at 01:17:14AM +0300, Dmitry Osipenko wrote: > > Use devm_platform_ioremap_resource() helper which makes code a bit > > cleaner. > > > > Signed-off-by: Dmitry Osipenko > > --- > > drivers/memory/tegra/tegra20-emc.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > I'm not a fan of this helper, to be honest, because I think all the > churn that we've seen with the conversions isn't really worth the 1 or 2 > lines that it saves, but hey, looks like this is pretty broadly > accepted, so if Krzysztof likes it: > > Acked-by: Thierry Reding Such changes indeed do not bring much but still less local variables and -1 line. I am fine with them. They also save one error msg from devm_ioremap_resource() in case of platform_get_resource() failure. Best regards, Krzysztof ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 32/52] memory: tegra20-emc: Continue probing if timings are missing in device-tree
On Mon, Oct 26, 2020 at 01:17:15AM +0300, Dmitry Osipenko wrote: > EMC driver will become mandatory after turning it into interconnect > provider because interconnect users, like display controller driver, will > fail to probe using newer device-trees that have interconnect properties. > Thus make EMC driver to probe even if timings are missing in device-tree. Does it really have to be mandatory? Sounds like that's going to make it unnecessarily complicated to merge all of this. Is it complicated to make interconnect support optional in consumer drivers? Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vboxvideo: Unmap iomem on probe-failure and remove
Add missing pci_iounmap() calls to properly unmap the memory on probe-failure and remove. Reported-by: kernel test robot Reported-by: Dan Carpenter Signed-off-by: Hans de Goede --- drivers/gpu/drm/vboxvideo/vbox_main.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/vboxvideo/vbox_main.c b/drivers/gpu/drm/vboxvideo/vbox_main.c index d68d9bad7674..2eeb1d3be54a 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_main.c +++ b/drivers/gpu/drm/vboxvideo/vbox_main.c @@ -71,6 +71,8 @@ static void vbox_accel_fini(struct vbox_private *vbox) for (i = 0; i < vbox->num_crtcs; ++i) vbva_disable(>vbva_info[i], vbox->guest_pool, i); + + pci_iounmap(vbox->ddev.pdev, vbox->vbva_buffers); } /* Do we support the 4.3 plus mode hint reporting interface? */ @@ -124,19 +126,19 @@ int vbox_hw_init(struct vbox_private *vbox) vbox->guest_pool = devm_gen_pool_create(vbox->ddev.dev, 4, -1, "vboxvideo-accel"); if (!vbox->guest_pool) - return -ENOMEM; + goto out_unmap_guest_heap; ret = gen_pool_add_virt(vbox->guest_pool, (unsigned long)vbox->guest_heap, GUEST_HEAP_OFFSET(vbox), GUEST_HEAP_USABLE_SIZE, -1); if (ret) - return ret; + goto out_unmap_guest_heap; ret = hgsmi_test_query_conf(vbox->guest_pool); if (ret) { DRM_ERROR("vboxvideo: hgsmi_test_query_conf failed\n"); - return ret; + goto out_unmap_guest_heap; } /* Reduce available VRAM size to reflect the guest heap. */ @@ -148,23 +150,30 @@ int vbox_hw_init(struct vbox_private *vbox) if (!have_hgsmi_mode_hints(vbox)) { ret = -ENOTSUPP; - return ret; + goto out_unmap_guest_heap; } vbox->last_mode_hints = devm_kcalloc(vbox->ddev.dev, vbox->num_crtcs, sizeof(struct vbva_modehint), GFP_KERNEL); - if (!vbox->last_mode_hints) - return -ENOMEM; + if (!vbox->last_mode_hints) { + ret = -ENOMEM; + goto out_unmap_guest_heap; + } ret = vbox_accel_init(vbox); if (ret) - return ret; + goto out_unmap_guest_heap; return 0; + +out_unmap_guest_heap: + pci_iounmap(vbox->ddev.pdev, vbox->guest_pool); + return ret; } void vbox_hw_fini(struct vbox_private *vbox) { vbox_accel_fini(vbox); + pci_iounmap(vbox->ddev.pdev, vbox->guest_pool); } -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 31/52] memory: tegra20-emc: Use devm_platform_ioremap_resource()
On Mon, Oct 26, 2020 at 01:17:14AM +0300, Dmitry Osipenko wrote: > Use devm_platform_ioremap_resource() helper which makes code a bit > cleaner. > > Signed-off-by: Dmitry Osipenko > --- > drivers/memory/tegra/tegra20-emc.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) I'm not a fan of this helper, to be honest, because I think all the churn that we've seen with the conversions isn't really worth the 1 or 2 lines that it saves, but hey, looks like this is pretty broadly accepted, so if Krzysztof likes it: Acked-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 30/52] memory: tegra20-emc: Make driver modular
On Mon, Oct 26, 2020 at 01:17:13AM +0300, Dmitry Osipenko wrote: > This patch adds modularization support to the Tegra20 EMC driver. Driver > now can be compiled as a loadable kernel module. > > Signed-off-by: Dmitry Osipenko > --- > drivers/memory/tegra/Kconfig | 2 +- > drivers/memory/tegra/tegra20-emc.c | 17 - > 2 files changed, 13 insertions(+), 6 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 29/52] memory: tegra-mc: Add interconnect framework
On Mon, Oct 26, 2020 at 01:17:12AM +0300, Dmitry Osipenko wrote: > Now Memory Controller is a memory interconnection provider. This allows > us to use interconnect API for tuning of memory configuration. This patch > adds common ICC core and adds hooks which should be implemented by the SoC > drivers. > > Tested-by: Peter Geis > Tested-by: Nicolas Chauvet > Signed-off-by: Dmitry Osipenko > --- > drivers/memory/tegra/Kconfig | 1 + > drivers/memory/tegra/mc.c| 129 +++ > drivers/memory/tegra/mc.h| 8 +++ > include/soc/tegra/mc.h | 16 + > 4 files changed, 154 insertions(+) > > diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig > index 9f0a96bf9ccc..b38e5255effe 100644 > --- a/drivers/memory/tegra/Kconfig > +++ b/drivers/memory/tegra/Kconfig > @@ -3,6 +3,7 @@ config TEGRA_MC > bool "NVIDIA Tegra Memory Controller support" > default y > depends on ARCH_TEGRA > + select INTERCONNECT > help > This driver supports the Memory Controller (MC) hardware found on > NVIDIA Tegra SoCs. > diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c > index 12ea2c79205a..53d61b05ebf8 100644 > --- a/drivers/memory/tegra/mc.c > +++ b/drivers/memory/tegra/mc.c > @@ -639,6 +639,133 @@ static __maybe_unused irqreturn_t tegra20_mc_irq(int > irq, void *data) > return IRQ_HANDLED; > } > > +static struct icc_node_data * > +tegra_mc_of_icc_xlate_extended(struct of_phandle_args *spec, void *data) > +{ > + struct icc_provider *provider = data; > + unsigned int idx = spec->args[0]; > + struct icc_node_data *ndata; > + struct icc_node *node; > + > + list_for_each_entry(node, >nodes, node_list) { > + if (node->id != idx) > + continue; > + > + ndata = kzalloc(sizeof(*ndata), GFP_KERNEL); > + if (!ndata) > + return ERR_PTR(-ENOMEM); > + > + ndata->node = node; > + > + /* these clients are isochronous by default on all SoCs */ > + if (strstarts(node->name, "display") || > + strstarts(node->name, "ptc") || > + strstarts(node->name, "vi")) > + ndata->tag = TEGRA_MC_ICC_TAG_ISO; This looks like something that might be better left to the drivers to decide. Doing this here seems okay for now, but I suspect that this will get fairly complicated to keep accurate as we add more clients later on. > + > + return ndata; > + } > + > + pr_err("%s: invalid client index %u\n", __func__, idx); Perhaps use "dev_err(provider->dev, ...);"? > + > + return ERR_PTR(-EINVAL); > +} > + > +/* > + * Memory Controller (MC) has few Memory Clients that are issuing memory > + * bandwidth allocation requests to the MC interconnect provider. The MC > + * provider aggregates the requests and then sends the aggregated request > + * up to the External Memory Controller (EMC) interconnect provider which > + * re-configures hardware interface to External Memory (EMEM) in accordance > + * to the required bandwidth. Each MC interconnect node represents an > + * individual Memory Client. > + * > + * Memory interconnect topology: > + * > + * ++ > + * ++|| > + * | TEXSRD +--->+| > + * ++|| > + * ||+-++--+ > + *...| MC +--->+ EMC +--->+ EMEM | > + * ||+-++--+ > + * ++|| > + * | DISP.. +--->+| > + * ++|| > + * ++ > + */ > +static int tegra_mc_interconnect_setup(struct tegra_mc *mc) > +{ > + struct icc_node *node; > + unsigned int i; > + int err; > + > + /* older device-trees don't have interconnect properties */ > + if (!of_find_property(mc->dev->of_node, "#interconnect-cells", NULL) || > + !mc->soc->icc_ops) > + return 0; This indicates that this property is indeed optional, so the bindings should reflect that. > + > + mc->provider.dev = mc->dev; > + mc->provider.data = >provider; > + mc->provider.set = mc->soc->icc_ops->set; > + mc->provider.aggregate = mc->soc->icc_ops->aggregate; > + mc->provider.xlate_extended = tegra_mc_of_icc_xlate_extended; > + > + err = icc_provider_add(>provider); > + if (err) > + goto err_msg; > + > + /* create Memory Controller node */ > + node = icc_node_create(TEGRA_ICC_MC); > + err = PTR_ERR_OR_ZERO(node); > + if (err) > + goto del_provider; > + > + node->name = "Memory Controller"; > + icc_node_add(node, >provider); > + > + /* link Memory Controller to External Memory Controller */ > + err = icc_link_create(node, TEGRA_ICC_EMC); > + if (err) > + goto remove_nodes; > + > + for (i = 0; i < mc->soc->num_clients; i++) { > + /* create MC client node */ > +
Re: [PATCH v6 28/52] memory: tegra: Add and use devm_tegra_get_memory_controller()
On Mon, Oct 26, 2020 at 01:17:11AM +0300, Dmitry Osipenko wrote: > Multiple Tegra drivers need to retrieve Memory Controller and there is > duplication of the retrieval code among the drivers. This patch removes > the duplication and fixes put_device() which was missed in the duplicated > code. > > EMC drivers now use new common devm_tegra_get_memory_controller() helper > instead of opencoding the MC retrieval. > > Signed-off-by: Dmitry Osipenko > --- > drivers/memory/tegra/mc.c| 48 > drivers/memory/tegra/tegra124-emc.c | 18 ++--- > drivers/memory/tegra/tegra210-emc-core.c | 39 +-- > drivers/memory/tegra/tegra30-emc.c | 18 ++--- > include/soc/tegra/mc.h | 10 + > 5 files changed, 74 insertions(+), 59 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 21/52] ARM: tegra: Add interconnect properties to Tegra20 device-tree
On Tue, Oct 27, 2020 at 10:12:47AM +0100, Krzysztof Kozlowski wrote: > On Mon, Oct 26, 2020 at 01:17:04AM +0300, Dmitry Osipenko wrote: > > Add interconnect properties to the Memory Controller, External Memory > > Controller and the Display Controller nodes in order to describe hardware > > interconnection. > > > > Signed-off-by: Dmitry Osipenko > > --- > > arch/arm/boot/dts/tegra20.dtsi | 26 +- > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi > > index 9347f7789245..2e1304493f7d 100644 > > --- a/arch/arm/boot/dts/tegra20.dtsi > > +++ b/arch/arm/boot/dts/tegra20.dtsi > > @@ -111,6 +111,17 @@ dc@5420 { > > > > nvidia,head = <0>; > > > > + interconnects = < TEGRA20_MC_DISPLAY0A >, > > I think you just added the defines and did not include them here, so > this should not even build. Did you test it? The dt-bindings/memory/tegra20-mc.h header is already included in existing DTS files for MC hot flush resets, so this should be fine. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 08/52] dt-bindings: memory: tegra20: emc: Document mfd-simple compatible and statistics sub-device
On Mon, Oct 26, 2020 at 01:16:51AM +0300, Dmitry Osipenko wrote: > External Memory Controller can gather various hardware statistics that > are intended to be used for debugging purposes and for dynamic frequency > scaling of memory bus. > > Document the new mfd-simple compatible and EMC statistics sub-device. > The subdev contains EMC DFS OPP table and interconnect paths to be used > for dynamic scaling of system's memory bandwidth based on EMC utilization > statistics. > > Signed-off-by: Dmitry Osipenko > --- > .../memory-controllers/nvidia,tegra20-emc.txt | 43 +-- > 1 file changed, 40 insertions(+), 3 deletions(-) Why does this have to be modelled as a separate device? Isn't this just using a couple of registers out of the EMC register range? If so, this would better just be integrated into the parent node and implemented as part of the EMC driver. No need to further complicate things by adding a dummy child. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 03/52] dt-bindings: memory: tegra20: emc: Correct registers range in example
On Mon, Oct 26, 2020 at 01:16:46AM +0300, Dmitry Osipenko wrote: > There is superfluous zero in the registers base address and registers > size should be twice bigger. > > Signed-off-by: Dmitry Osipenko > --- > .../bindings/memory-controllers/nvidia,tegra20-emc.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 02/52] soc/tegra: fuse: Export tegra_read_ram_code()
On Mon, Oct 26, 2020 at 01:16:45AM +0300, Dmitry Osipenko wrote: > The tegra_read_ram_code() is used by EMC drivers and we're going to make > these driver modular, hence this function needs to be exported. > > Signed-off-by: Dmitry Osipenko > --- > drivers/soc/tegra/fuse/tegra-apbmisc.c | 2 ++ > 1 file changed, 2 insertions(+) I'm not a big fan of exporting yet another of these tiny helpers, but I don't have any better ideas, so: Acked-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 01/52] clk: tegra: Export Tegra20 EMC kernel symbols
On Mon, Oct 26, 2020 at 01:16:44AM +0300, Dmitry Osipenko wrote: > We're going to modularize Tegra EMC drivers and some of the EMC clk driver > symbols need to be exported, let's export them. > > Signed-off-by: Dmitry Osipenko > --- > drivers/clk/tegra/clk-tegra20-emc.c | 3 +++ > 1 file changed, 3 insertions(+) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/9] drm/ttm: new TT backend allocation pool v2
On Mon, Oct 26, 2020 at 06:41:09PM +0100, Christian König wrote: > This replaces the spaghetti code in the two existing page pools. > > First of all depending on the allocation size it is between 3 (1GiB) and > 5 (1MiB) times faster than the old implementation. > > It makes better use of buddy pages to allow for larger physical contiguous > allocations which should result in better TLB utilization at least for > amdgpu. > > Instead of a completely braindead approach of filling the pool with one > CPU while another one is trying to shrink it we only give back freed > pages. > > This also results in much less locking contention and a trylock free MM > shrinker callback, so we can guarantee that pages are given back to the > system when needed. > > Downside of this is that it takes longer for many small allocations until > the pool is filled up. We could address this, but I couldn't find an use > case where this actually matters. We also don't bother freeing large > chunks of pages any more since the CPU overhead in that path isn't really > that important. > > The sysfs files are replaced with a single module parameter, allowing > users to override how many pages should be globally pooled in TTM. This > unfortunately breaks the UAPI slightly, but as far as we know nobody ever > depended on this. > > Zeroing memory coming from the pool was handled inconsistently. The > alloc_pages() based pool was zeroing it, the dma_alloc_attr() based one > wasn't. For now the new implementation isn't zeroing pages from the pool > either and only sets the __GFP_ZERO flag when necessary. > > The implementation has only 768 lines of code compared to the over 2600 > of the old one, and also allows for saving quite a bunch of code in the > drivers since we don't need specialized handling there any more based on > kernel config. > > Additional to all of that there was a neat bug with IOMMU, coherent DMA > mappings and huge pages which is now fixed in the new code as well. > > v2: make ttm_pool_apply_caching static as reported by the kernel bot, add > some more checks > > Signed-off-by: Christian König > --- > drivers/gpu/drm/ttm/Makefile | 2 +- > drivers/gpu/drm/ttm/ttm_memory.c | 3 + > drivers/gpu/drm/ttm/ttm_pool.c | 668 +++ > include/drm/ttm/ttm_caching.h| 2 + > include/drm/ttm/ttm_pool.h | 90 + > 5 files changed, 764 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/ttm/ttm_pool.c > create mode 100644 include/drm/ttm/ttm_pool.h > > diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile > index 90c0da88cc98..0096bacbcf32 100644 > --- a/drivers/gpu/drm/ttm/Makefile > +++ b/drivers/gpu/drm/ttm/Makefile > @@ -5,7 +5,7 @@ > ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \ > ttm_bo_util.o ttm_bo_vm.o ttm_module.o \ > ttm_execbuf_util.o ttm_page_alloc.o ttm_range_manager.o \ > - ttm_resource.o > + ttm_resource.o ttm_pool.o > ttm-$(CONFIG_AGP) += ttm_agp_backend.o > ttm-$(CONFIG_DRM_TTM_DMA_PAGE_POOL) += ttm_page_alloc_dma.o > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c > b/drivers/gpu/drm/ttm/ttm_memory.c > index 69cf622e79e5..3012d0388c51 100644 > --- a/drivers/gpu/drm/ttm/ttm_memory.c > +++ b/drivers/gpu/drm/ttm/ttm_memory.c > @@ -38,6 +38,7 @@ > #include > #include > #include > +#include > > #define TTM_MEMORY_ALLOC_RETRIES 4 > > @@ -453,6 +454,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) > } > ttm_page_alloc_init(glob, glob->zone_kernel->max_mem/(2*PAGE_SIZE)); > ttm_dma_page_alloc_init(glob, glob->zone_kernel->max_mem/(2*PAGE_SIZE)); > + ttm_pool_mgr_init(glob->zone_kernel->max_mem/(2*PAGE_SIZE)); > return 0; > out_no_zone: > ttm_mem_global_release(glob); > @@ -467,6 +469,7 @@ void ttm_mem_global_release(struct ttm_mem_global *glob) > /* let the page allocator first stop the shrink work. */ > ttm_page_alloc_fini(); > ttm_dma_page_alloc_fini(); > + ttm_pool_mgr_fini(); > > flush_workqueue(glob->swap_queue); > destroy_workqueue(glob->swap_queue); > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c > new file mode 100644 > index ..d25712e3ad3b > --- /dev/null > +++ b/drivers/gpu/drm/ttm/ttm_pool.c > @@ -0,0 +1,668 @@ > +// SPDX-License-Identifier: GPL-2.0 OR MIT > +/* > + * Copyright 2020 Advanced Micro Devices, Inc. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be
Re: [PATCH 2/8] ALSA: pcm: use krealloc_array()
On Tue, 27 Oct 2020 13:17:19 +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski > > Use the helper that checks for overflows internally instead of manually > calculating the size of the new array. > > Signed-off-by: Bartosz Golaszewski Reviewed-by: Takashi Iwai thanks, Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 8/8] dma-buf: use krealloc_array()
Am 27.10.20 um 13:17 schrieb Bartosz Golaszewski: From: Bartosz Golaszewski Use the helper that checks for overflows internally instead of manually calculating the size of the new array. Signed-off-by: Bartosz Golaszewski Acked-by: Christian König --- drivers/dma-buf/sync_file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 5a5a1da01a00..2925ea03eef0 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -270,8 +270,8 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, fences[i++] = dma_fence_get(a_fences[0]); if (num_fences > i) { - nfences = krealloc(fences, i * sizeof(*fences), - GFP_KERNEL); + nfences = krealloc_array(fences, i, +sizeof(*fences), GFP_KERNEL); if (!nfences) goto err; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 07/16] drm/i915/hdcp: Enable Gen12 HDCP 1.4 DP MST support
On 2020-10-27 at 11:59:14 +0530, Shankar, Uma wrote: > > > > -Original Message- > > From: Anshuman Gupta > > Sent: Friday, October 23, 2020 5:51 PM > > To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > > Cc: seanp...@chromium.org; Nikula, Jani ; C, > > Ramalingam ; Li, Juston ; > > Shankar, Uma ; Gupta, Anshuman > > > > Subject: [PATCH v3 07/16] drm/i915/hdcp: Enable Gen12 HDCP 1.4 DP MST > > support > > > > Enable HDCP 1.4 over DP MST for Gen12. > > This also enable the stream encryption support for older generations, which > > was > > missing earlier. > > > > v2: > > - Added debug print for stream encryption. > > - Disable the hdcp on port after disabling last stream > > encryption. > > Don't see port disable here, Am I missing something. > > > > > Cc: Ramalingam C > > Signed-off-by: Anshuman Gupta > > --- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 10 ++--- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 46 ++--- > > 2 files changed, 35 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > index 16865b200062..f00e12fc83e8 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > @@ -826,13 +826,9 @@ static struct drm_connector > > *intel_dp_add_mst_connector(struct drm_dp_mst_topolo > > intel_attach_force_audio_property(connector); > > intel_attach_broadcast_rgb_property(connector); > > > > - > > - /* TODO: Figure out how to make HDCP work on GEN12+ */ > > - if (INTEL_GEN(dev_priv) < 12) { > > - ret = intel_dp_init_hdcp(dig_port, intel_connector); > > - if (ret) > > - DRM_DEBUG_KMS("HDCP init failed, skipping.\n"); > > - } > > + ret = intel_dp_init_hdcp(dig_port, intel_connector); > > + if (ret) > > + drm_dbg_kms(_priv->drm, "HDCP init failed, skipping.\n"); > > > > /* > > * Reuse the prop from the SST connector because we're diff --git > > a/drivers/gpu/drm/i915/display/intel_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > index 61252d4be3dd..46c9bd588db1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -612,7 +612,12 @@ int intel_hdcp_auth_downstream(struct intel_connector > > *connector) > > return ret; > > } > > > > -/* Implements Part 1 of the HDCP authorization procedure */ > > +/* > > + * Implements Part 1 of the HDCP authorization procedure. > > + * Authentication Part 1 steps for Multi-stream DisplayPort. > > + * Step 1. Auth Part 1 sequence on the driving MST Trasport Link. > > + * Step 2. Enable encryption for each stream that requires encryption. > > + */ > > static int intel_hdcp_auth(struct intel_connector *connector) { > > struct intel_digital_port *dig_port = > > intel_attached_dig_port(connector); > > @@ -766,10 +771,16 @@ static int intel_hdcp_auth(struct intel_connector > > *connector) > > return -ETIMEDOUT; > > } > > > > - /* > > -* XXX: If we have MST-connected devices, we need to enable encryption > > -* on those as well. > > -*/ > > + /* DP MST Auth Part 1 Step 2.a and Step 2.b */ > > + if (shim->stream_encryption) { > > + ret = shim->stream_encryption(dig_port, true); > > + if (ret) { > > + drm_err(_priv->drm, "Failed to enable HDCP 1.4 > > stream enc\n"); > > + return ret; > > + } > > + drm_dbg_kms(_priv->drm, "HDCP 1.4 tras %s stream > > encrypted\n", > > + transcoder_name(hdcp->stream_transcoder)); > > + } > > > > if (repeater_present) > > return intel_hdcp_auth_downstream(connector); > > @@ -790,19 +801,26 @@ static int _intel_hdcp_disable(struct intel_connector > > *connector) > > > > drm_dbg_kms(_priv->drm, "[%s:%d] HDCP is being disabled...\n", > > connector->base.name, connector->base.base.id); > > + /* > > +* Step 1: Deselect HDCP Multiplestream Bit. > > +* Step 2: poll for stream encryption status to be disable. > > +*/ > > The above comment should be inside the callback, doesn't add value here. stream_encryption call back is common for enable/disable stream encryption. probbaly it would be better to nuke the above comment in case not adds value here. Thanks, Anshuman. > > > + if (hdcp->shim->stream_encryption) { > > + ret = hdcp->shim->stream_encryption(dig_port, false); > > + if (ret) { > > + drm_err(_priv->drm, "Failed to disable HDCP 1.4 > > stream enc\n"); > > + return ret; > > + } > > + drm_dbg_kms(_priv->drm, "HDCP 1.4 trans %s stream > > encryption disabled\n", > > + transcoder_name(hdcp->stream_transcoder)); > > + } > > > > /* > > -* If there are other connectors on this
Re: [PATCH 1/9] drm/ttm: new TT backend allocation pool v2
On Mon, Oct 26, 2020 at 06:41:09PM +0100, Christian König wrote: > This replaces the spaghetti code in the two existing page pools. > > First of all depending on the allocation size it is between 3 (1GiB) and > 5 (1MiB) times faster than the old implementation. > > It makes better use of buddy pages to allow for larger physical contiguous > allocations which should result in better TLB utilization at least for > amdgpu. > > Instead of a completely braindead approach of filling the pool with one > CPU while another one is trying to shrink it we only give back freed > pages. > > This also results in much less locking contention and a trylock free MM > shrinker callback, so we can guarantee that pages are given back to the > system when needed. > > Downside of this is that it takes longer for many small allocations until > the pool is filled up. We could address this, but I couldn't find an use > case where this actually matters. We also don't bother freeing large > chunks of pages any more since the CPU overhead in that path isn't really > that important. > > The sysfs files are replaced with a single module parameter, allowing > users to override how many pages should be globally pooled in TTM. This > unfortunately breaks the UAPI slightly, but as far as we know nobody ever > depended on this. > > Zeroing memory coming from the pool was handled inconsistently. The > alloc_pages() based pool was zeroing it, the dma_alloc_attr() based one > wasn't. For now the new implementation isn't zeroing pages from the pool > either and only sets the __GFP_ZERO flag when necessary. > > The implementation has only 768 lines of code compared to the over 2600 > of the old one, and also allows for saving quite a bunch of code in the > drivers since we don't need specialized handling there any more based on > kernel config. > > Additional to all of that there was a neat bug with IOMMU, coherent DMA > mappings and huge pages which is now fixed in the new code as well. > > v2: make ttm_pool_apply_caching static as reported by the kernel bot, add > some more checks > > Signed-off-by: Christian König Just verified them in my renoir platform, the page faults are gone. Thanks! Series are Tested-by: Huang Rui > --- > drivers/gpu/drm/ttm/Makefile | 2 +- > drivers/gpu/drm/ttm/ttm_memory.c | 3 + > drivers/gpu/drm/ttm/ttm_pool.c | 668 +++ > include/drm/ttm/ttm_caching.h| 2 + > include/drm/ttm/ttm_pool.h | 90 + > 5 files changed, 764 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/ttm/ttm_pool.c > create mode 100644 include/drm/ttm/ttm_pool.h > > diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile > index 90c0da88cc98..0096bacbcf32 100644 > --- a/drivers/gpu/drm/ttm/Makefile > +++ b/drivers/gpu/drm/ttm/Makefile > @@ -5,7 +5,7 @@ > ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \ > ttm_bo_util.o ttm_bo_vm.o ttm_module.o \ > ttm_execbuf_util.o ttm_page_alloc.o ttm_range_manager.o \ > - ttm_resource.o > + ttm_resource.o ttm_pool.o > ttm-$(CONFIG_AGP) += ttm_agp_backend.o > ttm-$(CONFIG_DRM_TTM_DMA_PAGE_POOL) += ttm_page_alloc_dma.o > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c > b/drivers/gpu/drm/ttm/ttm_memory.c > index 69cf622e79e5..3012d0388c51 100644 > --- a/drivers/gpu/drm/ttm/ttm_memory.c > +++ b/drivers/gpu/drm/ttm/ttm_memory.c > @@ -38,6 +38,7 @@ > #include > #include > #include > +#include > > #define TTM_MEMORY_ALLOC_RETRIES 4 > > @@ -453,6 +454,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) > } > ttm_page_alloc_init(glob, glob->zone_kernel->max_mem/(2*PAGE_SIZE)); > ttm_dma_page_alloc_init(glob, glob->zone_kernel->max_mem/(2*PAGE_SIZE)); > + ttm_pool_mgr_init(glob->zone_kernel->max_mem/(2*PAGE_SIZE)); > return 0; > out_no_zone: > ttm_mem_global_release(glob); > @@ -467,6 +469,7 @@ void ttm_mem_global_release(struct ttm_mem_global *glob) > /* let the page allocator first stop the shrink work. */ > ttm_page_alloc_fini(); > ttm_dma_page_alloc_fini(); > + ttm_pool_mgr_fini(); > > flush_workqueue(glob->swap_queue); > destroy_workqueue(glob->swap_queue); > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c > new file mode 100644 > index ..d25712e3ad3b > --- /dev/null > +++ b/drivers/gpu/drm/ttm/ttm_pool.c > @@ -0,0 +1,668 @@ > +// SPDX-License-Identifier: GPL-2.0 OR MIT > +/* > + * Copyright 2020 Advanced Micro Devices, Inc. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject
Re: [PATCH v2 1/3] drm/radeon: Add new callback that exposes vddc
On Mon, 26 Oct 2020 at 23:53, Alex Deucher wrote: > > I don't see them on the mailing list. Are you sure they went out? > > Alex The original email in this chain is the v2, sent in the correct form. > > On Sat, Oct 24, 2020 at 1:47 PM Sandeep wrote: > > > > Hello, > > > > I've resent the patches in the correct format. Please review. > > > > - Sandeep > > > > On Fri, 9 Oct 2020 at 13:14, Sandeep Raghuraman > > wrote: > > > > > > This patch adds a callback for reporting vddc, to the dpm field of the > > > radeon_asic structure. > > > > > > Signed-off-by: Sandeep Raghuraman > > > > > > --- > > > drivers/gpu/drm/radeon/radeon.h | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/gpu/drm/radeon/radeon.h > > > b/drivers/gpu/drm/radeon/radeon.h > > > index cc4f58d16589..85a1cafdf303 100644 > > > --- a/drivers/gpu/drm/radeon/radeon.h > > > +++ b/drivers/gpu/drm/radeon/radeon.h > > > @@ -1992,6 +1992,7 @@ struct radeon_asic { > > > int (*get_fan_speed_percent)(struct radeon_device *rdev, > > > u32 *speed); > > > u32 (*get_current_sclk)(struct radeon_device *rdev); > > > u32 (*get_current_mclk)(struct radeon_device *rdev); > > > + u16 (*get_current_vddc)(struct radeon_device *rdev); > > > } dpm; > > > /* pageflipping */ > > > struct { > > > -- > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: hdmi: Avoid sleeping in atomic context
On Tue, 27 Oct 2020 11:15:58 +0100, Maxime Ripard wrote: > > When running the trigger hook, ALSA by default will take a spinlock, and > thus will run the trigger hook in atomic context. > > However, our HDMI driver will send the infoframes as part of the trigger > hook, and part of that process is to wait for a bit to be cleared for up to > 100ms. To be nicer to the system, that wait has some usleep_range that > interact poorly with the atomic context. > > There's several ways we can fix this, but the more obvious one is to make > ALSA take a mutex instead by setting the nonatomic flag on the DAI link. > That doesn't work though, since now the cyclic callback installed by the > dmaengine helpers in ALSA will take a mutex, while that callback is run by > dmaengine's virt-chan code in a tasklet where sleeping is not allowed > either. > > Given the delay we need to poll the bit for, changing the usleep_range for > a udelay and keep running it from a context where interrupts are disabled > is not really a good option either. > > However, we can move the infoframe setup code in the hw_params hook, like > is usually done in other HDMI controllers, that isn't protected by a > spinlock and thus where we can sleep. Infoframes will be sent on a regular > basis anyway, and since hw_params is where the audio parameters that end up > in the infoframes are setup, this also makes a bit more sense. > > Fixes: bb7d78568814 ("drm/vc4: Add HDMI audio support") > Suggested-by: Mark Brown > Signed-off-by: Maxime Ripard Reviewed-by: Takashi Iwai thanks, Takashi > --- > drivers/gpu/drm/vc4/vc4_hdmi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > index 74da7c00ecd0..ec3ba3ecd32a 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > @@ -1077,6 +1077,7 @@ static int vc4_hdmi_audio_hw_params(struct > snd_pcm_substream *substream, > struct snd_soc_dai *dai) > { > struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai); > + struct drm_encoder *encoder = _hdmi->encoder.base.base; > struct device *dev = _hdmi->pdev->dev; > u32 audio_packet_config, channel_mask; > u32 channel_map; > @@ -1136,6 +1137,8 @@ static int vc4_hdmi_audio_hw_params(struct > snd_pcm_substream *substream, > HDMI_WRITE(HDMI_AUDIO_PACKET_CONFIG, audio_packet_config); > vc4_hdmi_set_n_cts(vc4_hdmi); > > + vc4_hdmi_set_audio_infoframe(encoder); > + > return 0; > } > > @@ -1143,11 +1146,9 @@ static int vc4_hdmi_audio_trigger(struct > snd_pcm_substream *substream, int cmd, > struct snd_soc_dai *dai) > { > struct vc4_hdmi *vc4_hdmi = dai_to_hdmi(dai); > - struct drm_encoder *encoder = _hdmi->encoder.base.base; > > switch (cmd) { > case SNDRV_PCM_TRIGGER_START: > - vc4_hdmi_set_audio_infoframe(encoder); > vc4_hdmi->audio.streaming = true; > > if (vc4_hdmi->variant->phy_rng_enable) > -- > 2.26.2 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 41/52] memory: tegra124-emc: Use devm_platform_ioremap_resource()
On Mon, Oct 26, 2020 at 01:17:24AM +0300, Dmitry Osipenko wrote: > Use devm_platform_ioremap_resource() helper which makes code a bit > cleaner. Such cleanups (and few other in this patchset) should be at beginning of patchset or even as part of a separate one. I think there is not much stopping anyone from applying these... except that you put them in the middle of big dependency. Best regards, Krzysztof ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel