Re: Phyr Starter
On 1/10/22 11:34, Matthew Wilcox wrote:
TLDR: I want to introduce a new data type:
struct phyr {
phys_addr_t addr;
size_t len;
};
and use it to replace bio_vec as well as using it to replace the array
of struct pages used by get_user_pages() and friends.
---
This would certainly solve quite a few problems at once. Very compelling.
Zooming in on the pinning aspect for a moment: last time I attempted to
convert O_DIRECT callers from gup to pup, I recall wanting very much to
record, in each bio_vec, whether these pages were acquired via FOLL_PIN,
or some non-FOLL_PIN method. Because at the end of the IO, it is not
easy to disentangle which pages require put_page() and which require
unpin_user_page*().
And changing the bio_vec for *that* purpose was not really acceptable.
But now that you're looking to change it in a big way (and with some
spare bits avaiable...oohh!), maybe I can go that direction after all.
Or, are you looking at a design in which any phyr is implicitly FOLL_PIN'd
if it exists at all?
Or any other thoughts in this area are very welcome.
There are two distinct problems I want to address: doing I/O to memory
which does not have a struct page and efficiently doing I/O to large
blobs of physically contiguous memory, regardless of whether it has a
struct page. There are some other improvements which I regard as minor.
There are many types of memory that one might want to do I/O to that do
not have a struct page, some examples:
- Memory on a graphics card (or other PCI card, but gfx seems to be
the primary provider of DRAM on the PCI bus today)
- DAX, or other pmem (there are some fake pages today, but this is
mostly a workaround for the IO problem today)
- Guest memory being accessed from the hypervisor (KVM needs to
create structpages to make this happen. Xen doesn't ...)
All of these kinds of memories can be addressed by the CPU and so also
by a bus master. That is, there is a physical address that the CPU
can use which will address this memory, and there is a way to convert
that to a DMA address which can be programmed into another device.
There's no intent here to support memory which can be accessed by a
complex scheme like writing an address to a control register and then
accessing the memory through a FIFO; this is for memory which can be
accessed by DMA and CPU loads and stores.
For get_user_pages() and friends, we currently fill an array of struct
pages, each one representing PAGE_SIZE bytes. For an application that
is using 1GB hugepages, writing 2^18 entries is a significant overhead.
It also makes drivers hard to write as they have to recoalesce the
struct pages, even though the VM can tell it whether those 2^18 pages
are contiguous.
On the minor side, struct phyr can represent any mappable chunk of memory.
A bio_vec is limited to 2^32 bytes, while on 64-bit machines a phyr
can represent larger than 4GB. A phyr is the same size as a bio_vec
on 64 bit (16 bytes), and the same size for 32-bit with PAE (12 bytes).
It is smaller for 32-bit machines without PAE (8 bytes instead of 12).
Finally, it may be possible to stop using scatterlist to describe the
input to the DMA-mapping operation. We may be able to get struct
scatterlist down to just dma_address and dma_length, with chaining
handled through an enclosing struct.
I would like to see phyr replace bio_vec everywhere it's currently used.
I don't have time to do that work now because I'm busy with folios.
If someone else wants to take that on, I shall cheer from the sidelines.
I'm starting to wonder if I should jump in here, in order to get this
as a way to make the O_DIRECT conversion much cleaner. But let's see.
What I do intend to do is:
- Add an interface to gup.c to pin/unpin N phyrs
- Add a sg_map_phyrs()
This will take an array of phyrs and allocate an sg for them
- Whatever else I need to do to make one RDMA driver happy with
this scheme
At that point, I intend to stop and let others more familiar with this
area of the kernel continue the conversion of drivers.
P.S. If you've had the Prodigy song running through your head the whole
time you've been reading this email ... I'm sorry / You're welcome.
If people insist, we can rename this to phys_range or something boring,
but I quite like the spelling of phyr with the pronunciation of "fire".
A more conservative or traditional name might look like:
phys_vec (maintains some resemblance to what it's replacing)
phys_range
phys_addr_range
phyr is rather cool, but it also is awfully too close to "phys" for
reading comfort. And there is a lot to be said for self-descriptive names,
which phyr is not.
And of course, you're signing for another huge naming debate with Linus
if you go with the "cool" name here. :)
thanks,
--
John Hubbard
NVIDIA
Re: [PATCH 3/3] drm/atomic: Make private objs proper objects
On Mon, 10 Jan 2022, Ville Syrjälä wrote:
> On Fri, Dec 31, 2021 at 03:23:31PM +0200, Jani Nikula wrote:
>> On Wed, 12 Jul 2017, ville.syrj...@linux.intel.com wrote:
>> > From: Ville Syrjälä
>> >
>> > Make the atomic private object stuff less special by introducing proper
>> > base classes for the object and its state. Drivers can embed these in
>> > their own appropriate objects, after which these things will work
>> > exactly like the plane/crtc/connector states during atomic operations.
>> >
>> > v2: Reorder to not depend on drm_dynarray (Daniel)
>> >
>> > Cc: Dhinakaran Pandiyan
>> > Cc: Daniel Vetter
>> > Reviewed-by: Daniel Vetter #v1
>> > Signed-off-by: Ville Syrjälä
>>
>> Stumbled upon an old commit
>>
>> commit a4370c777406c2810e37fafd166ccddecdb2a60c
>> Author: Ville Syrjälä
>> Date: Wed Jul 12 18:51:02 2017 +0300
>>
>> drm/atomic: Make private objs proper objects
>>
>> which is this patch.
>>
>> > @@ -3050,8 +3043,7 @@ struct drm_dp_mst_topology_state
>> > *drm_atomic_get_mst_topology_state(struct drm_a
>> >struct drm_device *dev = mgr->dev;
>> >
>> >WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>> > - return drm_atomic_get_private_obj_state(state, mgr,
>> > - &mst_state_funcs);
>> > + return to_dp_mst_topology_state(drm_atomic_get_private_obj_state(state,
>> > &mgr->base));
>> > }
>> > EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
>>
>> I don't think this combines well with...
>>
>> > diff --git a/include/drm/drm_dp_mst_helper.h
>> > b/include/drm/drm_dp_mst_helper.h
>> > index 177ab6f86855..d55abb75f29a 100644
>> > --- a/include/drm/drm_dp_mst_helper.h
>> > +++ b/include/drm/drm_dp_mst_helper.h
>> > @@ -404,12 +404,17 @@ struct drm_dp_payload {
>> >int vcpi;
>> > };
>> >
>> > +#define to_dp_mst_topology_state(x) container_of(x, struct
>> > drm_dp_mst_topology_state, base)
>>
>> ...this in case of error pointers that
>> drm_atomic_get_private_obj_state() may return.
>
> offsetof(base)==0 so should work in practice.
Returning zeros is fine, but error pointers are another matter.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 1/2] arm64: dts: exynos: Link DSI panel at port@1 for TM2 board
+CC: dri-devel
On 10.01.2022 16:27, Jagan Teki wrote:
TM2 board DSI pipeline has input from MIC and output to
s6e3ha2 panel.
The existing pipeline has child nodes of ports, panel and
MIC is remote-endpoint reference of port@0 of ports.
Adding panel as another child node to DSI is unconventional
as pipeline has ports child. However it can be true if MIC
is added inside port node like this.
dsi {
compatible = "samsung,exynos5433-mipi-dsi";
#address-cells = <1>;
#size-cells = <0>;
port {
dsi_to_mic: endpoint {
remote-endpoint = <&mic_to_dsi>;
};
};
panel@0 {
compatible = "samsung,s6e3hf2";
reg = <0>;
vdd3-supply = <&ldo27_reg>;
vci-supply = <&ldo28_reg>;
reset-gpios = <&gpg0 0 GPIO_ACTIVE_LOW>;
enable-gpios = <&gpf1 5 GPIO_ACTIVE_HIGH>;
};
};
The above pipeline is proper but it requires the DSI input MIC
pipeline to update.
This patch is trying to add panel at port@1 so-that the entire
pipeline before to panel output is untouched.
Reported-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 16
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index aca01709fd29..e13210c8d7e0 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -53,6 +53,16 @@ &cmu_disp {
};
&dsi {
+ ports {
+ port@1 {
+ reg = <1>;
+
+ dsi_out_panel: endpoint {
+ remote-endpoint = <&dsi_in_panel>;
+ };
+ };
+ };
+
panel@0 {
compatible = "samsung,s6e3ha2";
reg = <0>;
@@ -60,6 +70,12 @@ panel@0 {
vci-supply = <&ldo28_reg>;
reset-gpios = <&gpg0 0 GPIO_ACTIVE_LOW>;
enable-gpios = <&gpf1 5 GPIO_ACTIVE_HIGH>;
+
+ port {
+ dsi_in_panel: endpoint {
+ remote-endpoint = <&dsi_out_panel>;
+ };
+ };
As I already wrote in Exynos thread, DSI host has already parent-child
relation with the panel - DSI host knows well who is connected to it.
Adding another links between them is redundant and has no value added.
I have already answered in Exynos thread[1] how could you deal with the
issue, you have.
[1]:
https://lore.kernel.org/dri-devel/e541c52b-9751-933b-5eac-783dd0ed9...@intel.com/
Regards
Andrzej
};
};
Re: [PATCH] dma-buf: Move sysfs work out of DMA-BUF export/release path
On Tue, Jan 11, 2022 at 7:02 AM Hridya Valsaraju wrote: > > On Sun, Jan 9, 2022 at 11:28 PM Christian König > wrote: > > > > Am 07.01.22 um 22:25 schrieb Hridya Valsaraju: > > > On Fri, Jan 7, 2022 at 10:17 AM Hridya Valsaraju > > > wrote: > > >> On Fri, Jan 7, 2022 at 2:22 AM Christian König > > >> wrote: > > >>> Am 06.01.22 um 20:04 schrieb Hridya Valsaraju: > > On Thu, Jan 6, 2022 at 12:59 AM Christian König > > wrote: > > > Am 05.01.22 um 00:51 schrieb Hridya Valsaraju: > > >> Recently, we noticed an issue where a process went into direct > > >> reclaim > > >> while holding the kernfs rw semaphore for sysfs in write(exclusive) > > >> mode. This caused processes who were doing DMA-BUF exports and > > >> releases > > >> to go into uninterruptible sleep since they needed to acquire the > > >> same > > >> semaphore for the DMA-BUF sysfs entry creation/deletion. In order to > > >> avoid > > >> blocking DMA-BUF export/release for an indeterminate amount of time > > >> while another process is holding the sysfs rw semaphore in exclusive > > >> mode, this patch moves the per-buffer sysfs file creation/deleteion > > >> to > > >> a kthread. > > > Well I absolutely don't think that this is justified. > > > > > > You adding tons of complexity here just to avoid the overhead of > > > creating the sysfs files while exporting DMA-bufs which is an > > > operation > > > which should be done exactly once in the lifecycle for the most common > > > use case. > > > > > > Please explain further why that should be necessary. > > Hi Christian, > > > > We noticed that the issue sometimes causes the exporting process to go > > to the uninterrupted sleep state for tens of milliseconds which > > unfortunately leads to user-perceptible UI jank. This is the reason > > why we are trying to move the sysfs entry creation and deletion out of > > the DMA-BUF export/release path. I will update the commit message to > > include the same in the next revision. > > >>> That is still not a justification for this change. The question is why > > >>> do you need that in the first place? > > >>> > > >>> Exporting a DMA-buf should be something would should be very rarely, > > >>> e.g. only at the start of an application. > > >> Hi Christian, > > >> > > >> Yes, that is correct. Noticeable jank caused by the issue is not > > >> present at all times and happens on UI transitions(for example during > > >> app launches and exits) when there are several DMA-BUFs being exported > > >> and released. This is especially true in the case of the camera app > > >> since it exports and releases a relatively larger number of DMA-BUFs > > >> during launch and exit respectively. > > > > Well, that sounds at least better than before. > > > > >> > > >> Regards, > > >> Hridya > > >> > > >>> So this strongly looks like you are trying to optimize for an use case > > >>> where we should probably rethink what userspace is doing here instead. > > > Hello Christian, > > > > > > If you don't mind, could you please elaborate on the above statement? > > > > The purpose of DMA-buf is to share a rather low number of buffers > > between different drivers and/or applications. > > > > For example with triple buffering we have three buffers shared between > > the camera driver and the display driver, same thing for use cases > > between rendering and display. > > > > So even when you have ~100 applications open your should not share more > > than ~300 DMA-buf handles and doing that should absolutely not cause any > > problems like you described above. > > > > Long story short when this affects your user experience then your user > > space is exporting *much* more buffers than expected. Especially since > > the sysfs overhead is completely negligible compared to the overhead > > drivers have when they export buffers. > > > > I do not think we can solve this issue from userspace since the > problem is not due to the overhead of sysfs creation/teardown itself. > The problem is that the duration of time for which the exporting > process would need to sleep waiting for the kernfs_rwsem semaphore is > undefined when the holder of the semaphore goes under direct reclaim. > Fsnotify events for sysfs files are also generated while holding the > same semaphore. > > This is also not a problem due to the high number of DMA-BUF > exports during launch time, as even a single export can be delayed for > an unpredictable amount of time. We cannot eliminate DMA-BUF exports > completely during app-launches and we are unfortunately seeing reports > of the exporting process occasionally sleeping long enough to cause > user-visible jankiness :( > > We also looked at whether any optimizations are possible from the > kernfs implementation side[1] but the semaphore is used quite extensively > and it looks like the best way forward would be to remove
RE: 回复: [PATCH] drm/ttm: Put BO in its memory manager's lru list
[Public]
Hi Christian,
Looks this patch still missed in 5.16 kernel. Is it intentional?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/ttm/ttm_bo.c?h=v5.16
Regards,
Guchun
-Original Message-
From: amd-gfx On Behalf Of Pan, Xinhui
Sent: Tuesday, November 9, 2021 9:16 PM
To: Koenig, Christian ; amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Subject: 回复: 回复: [PATCH] drm/ttm: Put BO in its memory manager's lru list
[AMD Official Use Only]
[AMD Official Use Only]
Actually this patch does not totally fix the mismatch of lru list with mem_type
as mem_type is changed in ->move() and lru list is changed after that.
During this small period, another eviction could still happed and evict this
mismatched BO from sMam(say, its lru list is on vram domain) to sMem.
发件人: Pan, Xinhui
发送时间: 2021年11月9日 21:05
收件人: Koenig, Christian; amd-...@lists.freedesktop.org
抄送: dri-devel@lists.freedesktop.org
主题: 回复: 回复: [PATCH] drm/ttm: Put BO in its memory manager's lru list
Yes, a stable tag is needed. vulkan guys say 5.14 hit this issue too.
I think that amdgpu_bo_move() does support copy from sysMem to sysMem correctly.
maybe something below is needed.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c83ef42ca702..aa63ae7ddf1e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -485,7 +485,8 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
bool evict,
}
if (old_mem->mem_type == TTM_PL_SYSTEM &&
(new_mem->mem_type == TTM_PL_TT ||
-new_mem->mem_type == AMDGPU_PL_PREEMPT)) {
+new_mem->mem_type == AMDGPU_PL_PREEMPT ||
+new_mem->mem_type == TTM_PL_SYSTEM)) {
ttm_bo_move_null(bo, new_mem);
goto out;
}
otherwise, amdgpu_move_blit() is called to do the system memory copy which use
a wrong address.
206 /* Map only what can't be accessed directly */
207 if (!tmz && mem->start != AMDGPU_BO_INVALID_OFFSET) {
208 *addr = amdgpu_ttm_domain_start(adev, mem->mem_type) +
209 mm_cur->start;
210 return 0;
211 }
line 208, *addr is zero. So when amdgpu_copy_buffer submit job with such addr,
page fault happens.
发件人: Koenig, Christian
发送时间: 2021年11月9日 20:35
收件人: Pan, Xinhui; amd-...@lists.freedesktop.org
抄送: dri-devel@lists.freedesktop.org
主题: Re: 回复: [PATCH] drm/ttm: Put BO in its memory manager's lru list
Mhm, I'm not sure what the rational behind that is.
Not moving the BO would make things less efficient, but should never cause a
crash.
Maybe we should add a CC: stable tag and push it to -fixes instead?
Christian.
Am 09.11.21 um 13:28 schrieb Pan, Xinhui:
> [AMD Official Use Only]
>
> I hit vulkan cts test hang with navi23.
>
> dmesg says gmc page fault with address 0x0, 0x1000, 0x2000
> And some debug log also says amdgu copy one BO from system Domain to system
> Domain which is really weird.
>
> 发件人: Koenig, Christian
> 发送时间: 2021年11月9日 20:20
> 收件人: Pan, Xinhui; amd-...@lists.freedesktop.org
> 抄送: dri-devel@lists.freedesktop.org
> 主题: Re: [PATCH] drm/ttm: Put BO in its memory manager's lru list
>
> Am 09.11.21 um 12:19 schrieb xinhui pan:
>> After we move BO to a new memory region, we should put it to the new
>> memory manager's lru list regardless we unlock the resv or not.
>>
>> Signed-off-by: xinhui pan
> Interesting find, did you trigger that somehow or did you just
> stumbled over it by reading the code?
>
> Patch is Reviewed-by: Christian König , I
> will pick that up for drm-misc-next.
>
> Thanks,
> Christian.
>
>> ---
>>drivers/gpu/drm/ttm/ttm_bo.c | 2 ++
>>1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>> b/drivers/gpu/drm/ttm/ttm_bo.c index f1367107925b..e307004f0b28
>> 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -701,6 +701,8 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
>>ret = ttm_bo_evict(bo, ctx);
>>if (locked)
>>ttm_bo_unreserve(bo);
>> + else
>> + ttm_bo_move_to_lru_tail_unlocked(bo);
>>
>>ttm_bo_put(bo);
>>return ret;
Re: [PATCH] dma-buf-map: Fix dot vs comma in example
Hi Am 11.01.22 um 01:33 schrieb Lucas De Marchi: Fix typo: separate arguments with comma rather than dot. Signed-off-by: Lucas De Marchi Thank you. It's in drm-misc-next. Best regards Thomas --- include/linux/dma-buf-map.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h index 278d489e4bdd..19fa0b5ae5ec 100644 --- a/include/linux/dma-buf-map.h +++ b/include/linux/dma-buf-map.h @@ -52,13 +52,13 @@ * *struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(0xdeadbeaf); * - * dma_buf_map_set_vaddr(&map. 0xdeadbeaf); + * dma_buf_map_set_vaddr(&map, 0xdeadbeaf); * * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem(). * * .. code-block:: c * - * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf); + * dma_buf_map_set_vaddr_iomem(&map, 0xdeadbeaf); * * Instances of struct dma_buf_map do not have to be cleaned up, but * can be cleared to NULL with dma_buf_map_clear(). Cleared mappings -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v3 00/10] drm: Make drivers to honour the nomodeset parameter
Hi
patches 6 to 10 are
Acked-by: Thomas Zimmermann
Best regards
Thomas
Am 22.12.21 um 09:28 schrieb Javier Martinez Canillas:
The nomodeset kernel command line parameter is used to prevent the KMS/DRM
drivers to be registered/probed. But only a few drivers implement support
for this and most DRM drivers just ignore it.
This patch series is a v3 to make DRM drivers to honour nomodeset. It is
posted as separate patches to make easier for drivers maintainers to ack
or pick them independently at their own pace.
The drm_module_{pci,platform}_driver() helper macros are added, which are
just wrappers around module_{pci,platform}_driver() but adding a check for
drm_firmware_drivers_only() and returning -ENODEV if that is true.
PCI and platform DRM drivers are then modified in the following patches to
make use of those macros.
Only KMS drivers will be ported to use these new macros, and only for PCI
and platform DRM drivers. A follow-up series might do the same for drivers
that are rendering-only and for USB/SPI/I2C devices, but it will need more
discussion to agree whether that's desirable or not.
Not all drivers were posted in v3 to avoid flooding the list with too many
patches. I'm only including the patches adding the macros and some patches
as an example of their usage.
I've built tested with 'make allmodconfig && make M=drivers/gpu/drm' but I
don't have hardware to test the drivers, so review/testing is appreciated.
Best regards,
Javier
Changes in v3:
- Include Thomas Zimmermann's patches in the series and rebase on top.
- Add collected Acked-by tags from v2.
Changes in v2:
- Add drm_module_{pci,platform}_driver() macros and put the check there
(Thomas Zimmermann).
- Use the drm_module_*_driver() macros if possible (Thomas Zimmermann).
- Leave the DRM drivers that don't set the DRIVER_MODESET driver feature
(Lucas Stach).
- Leave USB/SPI/I2C drivers and only include PCI and platform ones
(Noralf Trønnes).
- Add collected Reviewed-by tags
Javier Martinez Canillas (5):
drm: Provide platform module-init macro
drm/imx/dcss: Replace module initialization with DRM helpers
drm/komeda: Replace module initialization with DRM helpers
drm/arm/hdlcd: Replace module initialization with DRM helpers
drm/malidp: Replace module initialization with DRM helpers
Thomas Zimmermann (5):
drm: Provide PCI module-init macros
drm/ast: Replace module-init boiler-plate code with DRM helpers
drm/bochs: Replace module-init boiler-plate code with DRM helpers
drm/cirrus: Replace module-init boiler-plate code with DRM helpers
drm/hisilicon/hibmc: Replace module initialization with DRM helpers
Documentation/gpu/drm-internals.rst | 6 +
.../gpu/drm/arm/display/komeda/komeda_drv.c | 3 +-
drivers/gpu/drm/arm/hdlcd_drv.c | 3 +-
drivers/gpu/drm/arm/malidp_drv.c | 3 +-
drivers/gpu/drm/ast/ast_drv.c | 18 +--
.../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +-
drivers/gpu/drm/imx/dcss/dcss-drv.c | 3 +-
drivers/gpu/drm/tiny/bochs.c | 20 +--
drivers/gpu/drm/tiny/cirrus.c | 17 +--
include/drm/drm_module.h | 125 ++
10 files changed, 147 insertions(+), 54 deletions(-)
create mode 100644 include/drm/drm_module.h
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: Phyr Starter
Dropping some thoughts from the gpu driver perspective, feel free to tell me it's nonsense from the mm/block view :-) Generally I think we really, really need something like this that's across all subsystems and consistent. On Tue, Jan 11, 2022 at 1:41 AM Jason Gunthorpe wrote: > On Mon, Jan 10, 2022 at 07:34:49PM +, Matthew Wilcox wrote: > > Finally, it may be possible to stop using scatterlist to describe the > > input to the DMA-mapping operation. We may be able to get struct > > scatterlist down to just dma_address and dma_length, with chaining > > handled through an enclosing struct. > > Can you talk about this some more? IMHO one of the key properties of > the scatterlist is that it can hold huge amounts of pages without > having to do any kind of special allocation due to the chaining. > > The same will be true of the phyr idea right? > > > I would like to see phyr replace bio_vec everywhere it's currently used. > > I don't have time to do that work now because I'm busy with folios. > > If someone else wants to take that on, I shall cheer from the sidelines. > > What I do intend to do is: > > I wonder if we mixed things though.. > > IMHO there is a lot of optimization to be had by having a > datastructure that is expressly 'the physical pages underlying a > contiguous chunk of va' > > If you limit to that scenario then we can be more optimal because > things like byte granular offsets and size in the interior pages don't > need to exist. Every interior chunk is always aligned to its order and > we only need to record the order. At least from the gfx side of things only allowing page sized chunks makes a lot of sense. sg chains kinda feel wrong because they allow byte chunks (but really that's just not allowed), so quite some mismatch. If we go with page size I think hardcoding a PHYS_PAGE_SIZE KB(4) would make sense, because thanks to x86 that's pretty much the lowest common denominator that all hw (I know of at least) supports. Not having to fiddle with "which page size do we have" in driver code would be neat. It makes writing portable gup code in drivers just needlessly silly. > An overall starting offset and total length allow computing the slice > of the first/last entry. > > If the physical address is always aligned then we get 12 free bits > from the min 4k alignment and also only need to store order, not an > arbitary byte granular length. > > The win is I think we can meaningfully cover most common cases using > only 8 bytes per physical chunk. The 12 bits can be used to encode the > common orders (4k, 2M, 1G, etc) and some smart mechanism to get > another 16 bits to cover 'everything'. > > IMHO storage density here is quite important, we end up having to keep > this stuff around for a long time. > > I say this here, because I've always though bio_vec/etc are more > general than we actually need, being byte granular at every chunk. > > > - Add an interface to gup.c to pin/unpin N phyrs > > - Add a sg_map_phyrs() > >This will take an array of phyrs and allocate an sg for them > > - Whatever else I need to do to make one RDMA driver happy with > >this scheme > > I spent alot of time already cleaning all the DMA code in RDMA - it is > now nicely uniform and ready to do this sort of change. I was > expecting to be a bio_vec, but this is fine too. > > What is needed is a full scatterlist replacement, including the IOMMU > part. > > For the IOMMU I would expect the datastructure to be re-used, we start > with a list of physical pages and then 'dma map' gives us a list of > IOVA physical pages, in another allocation, but exactly the same > datastructure. > > This 'dma map' could return a pointer to the first datastructure if > there is no iommu, allocate a single entry list if the whole thing can > be linearly mapped with the iommu, and other baroque cases (like pci > offset/etc) will need to allocate full array. ie good HW runs fast and > is memory efficient. > > It would be nice to see a patch sketching showing what this > datastructure could look like. > > VFIO would like this structure as well as it currently is a very > inefficient page at a time loop when it iommu maps things. I also think that it would be nice if we can have this as a consistent set of datastructures, both for dma_addr_t and phys_addr_t. Roughly my wishlist: - chained by default, because of the allocation headaches, so I'm with Jason here - comes compressed out of gup, I think we all agree on this, I don't really care how it's compressed as long as I don't get 512 entries for 2M pages :-) - ideally the dma_ and the phys_ part are split since for dma-buf and some gpu driver internal use-case there's some where really only the dma addr is valid, and nothing else. But we can also continue the current hack of pretending the other side isn't there (atm we scramble those pointers to make sure dma-buf users don't cheat) - I think minimally an sg list form of dma-mapped stuff which does not have a struct page
[PATCH v3 1/1] drm/bridge: anx7625: send DPCD command to downstream
Send DPCD command to downstream before anx7625 power down,
let downstream monitor enter into standby mode.
Signed-off-by: Xin Ji
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 42 +++
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 33383f83255d..0b858c78abe8 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -129,6 +129,23 @@ static int anx7625_reg_write(struct anx7625_data *ctx,
return ret;
}
+static int anx7625_reg_block_write(struct anx7625_data *ctx,
+ struct i2c_client *client,
+ u8 reg_addr, u8 len, u8 *buf)
+{
+ int ret;
+ struct device *dev = &client->dev;
+
+ i2c_access_workaround(ctx, client);
+
+ ret = i2c_smbus_write_i2c_block_data(client, reg_addr, len, buf);
+ if (ret < 0)
+ dev_err(dev, "write i2c block failed id=%x\n:%x",
+ client->addr, reg_addr);
+
+ return ret;
+}
+
static int anx7625_write_or(struct anx7625_data *ctx,
struct i2c_client *client,
u8 offset, u8 mask)
@@ -214,8 +231,8 @@ static int wait_aux_op_finish(struct anx7625_data *ctx)
return 0;
}
-static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
-u32 address, u8 len, u8 *buf)
+static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx, u8 op,
+ u32 address, u8 len, u8 *buf)
{
struct device *dev = &ctx->client->dev;
int ret;
@@ -231,8 +248,7 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
addrm = (address >> 8) & 0xFF;
addrh = (address >> 16) & 0xFF;
- cmd = DPCD_CMD(len, DPCD_READ);
- cmd = ((len - 1) << 4) | 0x09;
+ cmd = DPCD_CMD(len, op);
/* Set command and length */
ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
@@ -246,6 +262,9 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
AP_AUX_ADDR_19_16, addrh);
+ if (op == DPCD_WRITE)
+ ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p0_client,
+ AP_AUX_BUFF_START, len, buf);
/* Enable aux access */
ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client,
AP_AUX_CTRL_STATUS, AP_AUX_CTRL_OP_EN);
@@ -255,14 +274,17 @@ static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
return -EIO;
}
- usleep_range(2000, 2100);
-
ret = wait_aux_op_finish(ctx);
if (ret) {
dev_err(dev, "aux IO error: wait aux op finish.\n");
return ret;
}
+ /* Write done */
+ if (op == DPCD_WRITE)
+ return 0;
+
+ /* Read done, read out dpcd data */
ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
AP_AUX_BUFF_START, len, buf);
if (ret < 0) {
@@ -845,7 +867,7 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx)
}
/* Read downstream capability */
- anx7625_aux_dpcd_read(ctx, 0x68028, 1, &bcap);
+ anx7625_aux_dpcd_trans(ctx, DPCD_READ, 0x68028, 1, &bcap);
if (!(bcap & 0x01)) {
pr_warn("downstream not support HDCP 1.4, cap(%x).\n", bcap);
return 0;
@@ -918,6 +940,7 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
{
struct device *dev = &ctx->client->dev;
int ret;
+ u8 data;
DRM_DEV_DEBUG_DRIVER(dev, "stop dp output\n");
@@ -929,6 +952,11 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
ret |= anx7625_write_and(ctx, ctx->i2c.tx_p2_client, 0x08, 0x7f);
ret |= anx7625_video_mute_control(ctx, 1);
+
+ dev_dbg(dev, "notify downstream enter into standby\n");
+ /* Downstream monitor enter into standby mode */
+ data = 2;
+ ret |= anx7625_aux_dpcd_trans(ctx, DPCD_WRITE, 0x000600, 1, &data);
if (ret < 0)
DRM_DEV_ERROR(dev, "IO error : mute video fail\n");
--
2.25.1
Re: [PATCH v1 1/2] drm/sprd: remove the selected DRM_KMS_CMA_HELPER in kconfig
Hello Kevin,
On 12/24/21 15:12, Kevin Tang wrote:
> On linux-next, commit 43531edd53f0 ("drm/sprd: add Unisoc's drm kms master")
> adds the config DRM_SPRD,
> which selects DRM_KMS_CMA_HELPER.
>
According to "The canonical patch format" section in [0], the body of the
explanation has to be line wrapped at 75 columns. But your sentences are
much longer than that.
[0]:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
> However, commit 09717af7d13d ("drm: Remove CONFIG_DRM_KMS_CMA_HELPER option")
> just removed the
> DRM_KMS_CMA_HELPER. So, the select DRM_KMS_CMA_HELPER refers to a
> non-existing kconfig symbol.
>
> Cc: Orson Zhai
> Cc: Chunyan Zhang
> Signed-off-by: Kevin Tang
> ---
Other than that, the patch looks good to me.
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH] i915: make array flex_regs static const
On 09/01/2022 20:31, Colin Ian King wrote:
Don't populate the read-only array flex_regs on the stack but
instead it static const. Also makes the object code a little smaller.
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/i915/i915_perf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index e27f3b7cf094..df698960fdc0 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2114,7 +2114,7 @@ gen8_update_reg_state_unlocked(const struct intel_context
*ce,
u32 ctx_oactxctrl = stream->perf->ctx_oactxctrl_offset;
u32 ctx_flexeu0 = stream->perf->ctx_flexeu0_offset;
/* The MMIO offsets for Flex EU registers aren't contiguous */
- i915_reg_t flex_regs[] = {
+ static const i915_reg_t flex_regs[] = {
EU_PERF_CNTL0,
EU_PERF_CNTL1,
EU_PERF_CNTL2,
Reviewed-by: Tvrtko Ursulin
And will merge shortly, thanks for the patch.
Regards,
Tvrtko
Re: [PATCH v1 2/2] drm/sprd: fix potential NULL dereference
On 12/24/21 15:12, Kevin Tang wrote: > platform_get_resource() may fail and return NULL, so check it's value > before using it. > > 'drm' could be null in sprd_drm_shutdown, and drm_warn maybe dereference > it, remove this warning log. > I would split this second change in a separate patch and just keep this one about checking the platform_get_resource() return value. If you do that, feel free to add: Reviewed-by: Javier Martinez Canillas Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH v1 1/2] drm/sprd: remove the selected DRM_KMS_CMA_HELPER in kconfig
Hi
Am 24.12.21 um 15:12 schrieb Kevin Tang:
On linux-next, commit 43531edd53f0 ("drm/sprd: add Unisoc's drm kms master")
adds the config DRM_SPRD,
which selects DRM_KMS_CMA_HELPER.
However, commit 09717af7d13d ("drm: Remove CONFIG_DRM_KMS_CMA_HELPER option")
just removed the
DRM_KMS_CMA_HELPER. So, the select DRM_KMS_CMA_HELPER refers to a non-existing
kconfig symbol.
With the long lines fixed, you can add
Acked-by: Thomas Zimmermann
Selecting DRM_GEM_CMA_HELPER and DRM_KMS_HELPER at the same time acts
like DRM_KMS_CMA_HELPER did before.
Best regards
Thomas
Cc: Orson Zhai
Cc: Chunyan Zhang
Signed-off-by: Kevin Tang
---
drivers/gpu/drm/sprd/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig
index 3edeaeca0..9a9c7ebfc 100644
--- a/drivers/gpu/drm/sprd/Kconfig
+++ b/drivers/gpu/drm/sprd/Kconfig
@@ -3,7 +3,6 @@ config DRM_SPRD
depends on ARCH_SPRD || COMPILE_TEST
depends on DRM && OF
select DRM_GEM_CMA_HELPER
- select DRM_KMS_CMA_HELPER
select DRM_KMS_HELPER
select DRM_MIPI_DSI
select VIDEOMODE_HELPERS
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v1 1/2] drm/sprd: remove the selected DRM_KMS_CMA_HELPER in kconfig
On Fri, Dec 24, 2021 at 3:12 PM Kevin Tang wrote:
>
> On linux-next, commit 43531edd53f0 ("drm/sprd: add Unisoc's drm kms master")
> adds the config DRM_SPRD,
> which selects DRM_KMS_CMA_HELPER.
>
That this commit is _currently_ on linux-next is just a matter of the
current state. The commit message that goes into the project's history
should probably not state "on linux-next";
this information is probably outdated or of no interest to any further
future reader at the time of reading.
So, just drop "On linux-next". The commit 43531edd53f0 ("drm/sprd: add
Unisoc's drm kms master") will exist until the end of time.
> However, commit 09717af7d13d ("drm: Remove CONFIG_DRM_KMS_CMA_HELPER option")
> just removed the
> DRM_KMS_CMA_HELPER. So, the select DRM_KMS_CMA_HELPER refers to a
> non-existing kconfig symbol.
>
I would be happy about acknowledging my work of reporting with a
Reported-by tag, although I accidently send the report only to you
without cc-ing the mailing lists.
Please add:
Reported-by: Lukas Bulwahn
That said you may also add a Reviewed-by tag now:
Reviewed-by: Lukas Bulwahn
Lukas
> Cc: Orson Zhai
> Cc: Chunyan Zhang
> Signed-off-by: Kevin Tang
> ---
> drivers/gpu/drm/sprd/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig
> index 3edeaeca0..9a9c7ebfc 100644
> --- a/drivers/gpu/drm/sprd/Kconfig
> +++ b/drivers/gpu/drm/sprd/Kconfig
> @@ -3,7 +3,6 @@ config DRM_SPRD
> depends on ARCH_SPRD || COMPILE_TEST
> depends on DRM && OF
> select DRM_GEM_CMA_HELPER
> - select DRM_KMS_CMA_HELPER
> select DRM_KMS_HELPER
> select DRM_MIPI_DSI
> select VIDEOMODE_HELPERS
> --
> 2.29.0
>
Re: [PATCH v10 0/5] group dp driver related patches into one series
On 10/01/2022 23:55, Kuogee Hsieh wrote: Group below 5 dp driver related patches into one series. Could you please rebase this on top of msm-next? Kuogee Hsieh (5): drm/msm/dp: dp_link_parse_sink_count() return immediately if aux read failed This patch is already a part of the tree. drm/msm/dp: do not initialize phy until plugin interrupt received drm/msm/dp: populate connector of struct dp_panel This one does not apply because of your dp-bridge patch. The conflict is more or less obvious to fix, but it would be nice to have the proper version from you. drm/msm/dp: add support of tps4 (training pattern 4) for HBR3 drm/msm/dp: stop link training after link training 2 failed drivers/gpu/drm/msm/dp/dp_catalog.c | 12 ++--- drivers/gpu/drm/msm/dp/dp_catalog.h | 2 +- drivers/gpu/drm/msm/dp/dp_ctrl.c| 100 drivers/gpu/drm/msm/dp/dp_ctrl.h| 8 +-- drivers/gpu/drm/msm/dp/dp_display.c | 98 --- drivers/gpu/drm/msm/dp/dp_link.c| 19 +-- 6 files changed, 140 insertions(+), 99 deletions(-) -- With best wishes Dmitry
Re: [PATCH RESEND v4 v5 4/4] drm/vc4: Notify the firmware when DRM is in charge
Hello Maxime, On Wed, Dec 15, 2021 at 10:51 AM Maxime Ripard wrote: > > Once the call to drm_fb_helper_remove_conflicting_framebuffers() has > been made, simplefb has been unregistered and the KMS driver is entirely > in charge of the display. > > Thus, we can notify the firmware it can free whatever resource it was > using to maintain simplefb functional. > > Signed-off-by: Maxime Ripard > --- Patch looks good to me. Reviewed-by: Javier Martinez Canillas Best regards, Javier
[RFC v3 0/7] Add GuC Error Capture Support
This series: 1. Supports the roll out of an upcoming GuC feature to enable error-state-capture that allows the driver to register lists of MMIO registers that GuC will report during a GuC triggered engine-reset event. 2. Updates the ADS blob creation to register lists of global and engine registers with GuC. 3. Defines tables of register lists that are global or engine class or engine instance in scope. 4. Separates GuC log-buffer access locks for relay logging vs the new region for the error state capture data. 5. Allocates an additional interim circular buffer store to copy snapshots of new GuC reported error-state-capture dumps in response to the G2H notification. 6. Connects the i915_gpu_coredump reporting function to the GuC error capture module to print all GuC error state capture dumps that is reported. This is the 3rd rev of this series. However, take note that this series will not pass CI until the following series is merged to upgrade the GuC firmware to a newer version which requires Patch #1 of this series is dropped: https://patchwork.kernel.org/project/intel-gfx/list/?series=599007 Prior receipts of rvb's: - Patch #4 has received R-v-b from Matthew Brost Changes from prior revs: v3: - Fixed all review comments from rev2 except the following: - Michal Wajdeczko proposed adding a seperate function to lookup register string nameslookup (based on offset) but decided against it because of offset conflicts and the current table layout is easier to maintain. - Last set of checkpatch errors pertaining to "COMPLEX MACROS" should be fixed on next rev. - Abstracted internal-to-guc-capture information into a new __guc_state_capture_priv structure that allows the exclusion of intel_guc.h and intel_guc_fwif.h from intel_guc_capture.h. Now, only the first 2 patches have a wider build time impact because of the changes to intel_guc_fwif.h but subsequent changes to guc-capture internal structures or firmware interfaces used solely by guc-capture module shoudn't impact the rest of the driver build. - Added missing Gen12LP registers and added slice+subslice indices when reporting extended steered registers. - Add additional checks to ensure that the GuC reported error capture information matches the i915_gpu_coredump that is being printed before we print out the corresponding VMA dumps such as the batch buffer. v2: - Ignore - failed CI retest. Alan Previn (6): drm/i915/guc: Update GuC ADS size for error capture lists drm/i915/guc: Populate XE_LP register lists for GuC error state capture. drm/i915/guc: Add GuC's error state capture output structures. drm/i915/guc: Update GuC's log-buffer-state access for error capture. drm/i915/guc: Copy new GuC error capture logs upon G2H notification. drm/i915/guc: Print the GuC error capture output register list. John Harrison (1): drm/i915/guc: Add basic support for error capture lists drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/gt/intel_engine_cs.c |4 +- .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h |8 + drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h | 85 ++ drivers/gpu/drm/i915/gt/uc/intel_guc.c| 55 +- drivers/gpu/drm/i915/gt/uc/intel_guc.h| 15 +- drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 32 +- .../gpu/drm/i915/gt/uc/intel_guc_capture.c| 1274 + .../gpu/drm/i915/gt/uc/intel_guc_capture.h| 30 + drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c |3 + drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 43 +- drivers/gpu/drm/i915/gt/uc/intel_guc_log.c| 162 ++- drivers/gpu/drm/i915/gt/uc/intel_guc_log.h| 23 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 22 + drivers/gpu/drm/i915/i915_gpu_error.c | 65 +- drivers/gpu/drm/i915/i915_gpu_error.h | 14 + 16 files changed, 1731 insertions(+), 105 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/uc/guc_capture_fwif.h create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h -- 2.25.1
Re: [PATCH v1 2/2] drm/sprd: fix potential NULL dereference
Hi,
on the changes for platform_get_resource(), you can
Acked-by: Thomas Zimmermann
but see my comments below.
Am 24.12.21 um 15:12 schrieb Kevin Tang:
platform_get_resource() may fail and return NULL, so check it's value
before using it.
'drm' could be null in sprd_drm_shutdown, and drm_warn maybe dereference
it, remove this warning log.
Cc: Orson Zhai
Cc: Chunyan Zhang
Signed-off-by: Kevin Tang
---
drivers/gpu/drm/sprd/sprd_dpu.c | 3 +++
drivers/gpu/drm/sprd/sprd_drm.c | 8 ++--
drivers/gpu/drm/sprd/sprd_dsi.c | 3 +++
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/sprd/sprd_dpu.c b/drivers/gpu/drm/sprd/sprd_dpu.c
index 06a3414ee..69683b7ba 100644
--- a/drivers/gpu/drm/sprd/sprd_dpu.c
+++ b/drivers/gpu/drm/sprd/sprd_dpu.c
@@ -790,6 +790,9 @@ static int sprd_dpu_context_init(struct sprd_dpu *dpu,
int ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
+
You can add an error message if this fails.
ctx->base = devm_ioremap(dev, res->start, resource_size(res));
if (!ctx->base) {
dev_err(dev, "failed to map dpu registers\n");
diff --git a/drivers/gpu/drm/sprd/sprd_drm.c b/drivers/gpu/drm/sprd/sprd_drm.c
index a077e2d4d..54030839e 100644
--- a/drivers/gpu/drm/sprd/sprd_drm.c
+++ b/drivers/gpu/drm/sprd/sprd_drm.c
@@ -154,12 +154,8 @@ static void sprd_drm_shutdown(struct platform_device *pdev)
{
struct drm_device *drm = platform_get_drvdata(pdev);
- if (!drm) {
- drm_warn(drm, "drm device is not available, no shutdown\n");
- return;
- }
-
- drm_atomic_helper_shutdown(drm);
+ if (drm)
+ drm_atomic_helper_shutdown(drm);
This change should be in a separate patch. Instead of removing the
warning, you should rather use dev_err() or dev_warn() from [1]. Not
being able to shut down here looks like a serious driver bug that the
user should know about.
}
static const struct of_device_id drm_match_table[] = {
diff --git a/drivers/gpu/drm/sprd/sprd_dsi.c b/drivers/gpu/drm/sprd/sprd_dsi.c
index 911b3cddc..955c5995a 100644
--- a/drivers/gpu/drm/sprd/sprd_dsi.c
+++ b/drivers/gpu/drm/sprd/sprd_dsi.c
@@ -907,6 +907,9 @@ static int sprd_dsi_context_init(struct sprd_dsi *dsi,
struct resource *res;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
+
Again, an error message seems appropriate.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/latest/source/include/linux/dev_printk.h#L145
ctx->base = devm_ioremap(dev, res->start, resource_size(res));
if (!ctx->base) {
drm_err(dsi->drm, "failed to map dsi host registers\n");
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v4 0/6] drm: exynos: dsi: Convert drm bridge
Hi Andrzej,
On Tue, Dec 28, 2021 at 4:18 PM Andrzej Hajda wrote:
>
> Hi Marek,
>
> On 23.12.2021 10:15, Marek Szyprowski wrote:
> > Hi Jagan,
> >
> > On 18.12.2021 00:16, Marek Szyprowski wrote:
> >> On 15.12.2021 15:56, Jagan Teki wrote:
> >>> On Wed, Dec 15, 2021 at 7:49 PM Marek Szyprowski
> >>> wrote:
> On 15.12.2021 13:57, Jagan Teki wrote:
> > On Wed, Dec 15, 2021 at 5:31 PM Marek Szyprowski
> > wrote:
> >> On 15.12.2021 11:15, Jagan Teki wrote:
> >>> Updated series about drm bridge conversion of exynos dsi.
> >>> Previous version can be accessible, here [1].
> >>>
> >>> Patch 1: connector reset
> >>>
> >>> Patch 2: panel_bridge API
> >>>
> >>> Patch 3: Bridge conversion
> >>>
> >>> Patch 4: Atomic functions
> >>>
> >>> Patch 5: atomic_set
> >>>
> >>> Patch 6: DSI init in enable
> >> There is a little progress! :)
> >>
> >> Devices with a simple display pipeline (only a DSI panel, like
> >> Trats/Trats2) works till the last patch. Then, after applying
> >> ("[PATCH
> >> v4 6/6] drm: exynos: dsi: Move DSI init in bridge enable"), I get no
> >> display at all.
> >>
> >> A TM2e board with in-bridge (Exynos MIC) stops displaying anything
> >> after
> >> applying patch "[PATCH v4 2/6] drm: exynos: dsi: Use drm
> >> panel_bridge API".
> >>
> >> In case of the Arndale board with tc358764 bridge, no much
> >> progress. The
> >> display is broken just after applying the "[PATCH v2] drm: bridge:
> >> tc358764: Use drm panel_bridge API" patch on top of linux-next.
> >>
> >> In all cases the I had "drm: of: Lookup if child node has panel or
> >> bridge" patch applied.
> > Just skip the 6/6 for now.
> >
> > Apply
> > -
> > https://protect2.fireeye.com/v1/url?k=a24f3f76-fdd40659-a24eb439-0cc47a31cdf8-97ea12b4c5258d11&q=1&e=37a169bf-7ca5-4362-aad7-486018c7a708&u=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fpatch%2F1825%2F
> > -
> > https://protect2.fireeye.com/v1/url?k=a226360f-fdbd0f20-a227bd40-0cc47a31cdf8-ebd66aebee1058d7&q=1&e=37a169bf-7ca5-4362-aad7-486018c7a708&u=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fpatch%2F1823%2F
> >
> > Then apply 1/6 to 5/6. and update the status?
> Okay, my fault, I didn't check that case on Arndale.
>
> I've checked and indeed, Trats/Trats2 and Arndale works after the above
> 2 patches AND patches 1-5.
>
> The only problem is now on TM2e, which uses Exynos MIC as in-bridge for
> Exynos DSI:
>
> [4.068866] [drm] Exynos DRM: using 1380.decon device for DMA
> mapping operations
> [4.069183] exynos-drm exynos-drm: bound 1380.decon (ops
> decon_component_ops)
> [4.128983] exynos-drm exynos-drm: bound 1388.decon (ops
> decon_component_ops)
> [4.129261] exynos-drm exynos-drm: bound 1393.mic (ops
> exynos_mic_component_ops)
> [4.133508] exynos-dsi 1390.dsi: [drm:exynos_dsi_host_attach]
> *ERROR* failed to find the bridge: -19
> [4.136392] exynos-drm exynos-drm: bound 1390.dsi (ops
> exynos_dsi_component_ops)
> [4.145499] rc_core: Couldn't load IR keymap rc-cec
> [4.145666] Registered IR keymap rc-empty
> [4.148402] rc rc0: sii8620 as /devices/virtual/rc/rc0
> [4.156051] input: sii8620 as /devices/virtual/rc/rc0/input1
> [4.160647] exynos-drm exynos-drm: bound 1397.hdmi (ops
> hdmi_component_ops)
> [4.169923] exynos-drm exynos-drm: [drm] Cannot find any crtc or
> sizes
> [4.173958] exynos-drm exynos-drm: [drm] Cannot find any crtc or
> sizes
> [4.182304] [drm] Initialized exynos 1.1.0 20180330 for
> exynos-drm on
> minor 0
>
> The display pipeline for TM2e is:
>
> Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel
> >>> If Trats/Trats2 is working then it has to work. I don't see any
> >>> difference in output pipeline. Can you please share the full log, I
> >>> cannot see host_attach print saying "Attached.."
> >> Well, there is a failure message about the panel:
> >>
> >> exynos-dsi 1390.dsi: [drm:exynos_dsi_host_attach] *ERROR* failed
> >> to find the bridge: -19
> >>
> >> however it looks that something might be broken in dts. The in-bridge
> >> (Exynos MIC) is on port 0 and the panel is @0, what imho might cause
> >> the issue.
> >>
> >> I've tried to change in in-bridge ('mic_to_dsi') port to 1 in
> >> exynos5433.dtsi. Then the panel has been attached:
> >>
> >> exynos-dsi 1390.dsi: [drm:exynos_dsi_host_attach] Attached s6e3hf2
> >> device
> >>
> >> but the display is still not working, probably due to lack of proper
> >> Exynos MIC handling. I will investigate it later and let You know.
> >
> > I've played a bit with the Exynos DRM code and finally I made it working
> > on TM2(e). There are basically
Re: [PATCH 1/2] arm64: dts: exynos: Link DSI panel at port@1 for TM2 board
Hi Andrzej,
On Tue, Jan 11, 2022 at 2:05 PM Andrzej Hajda wrote:
>
> +CC: dri-devel
>
> On 10.01.2022 16:27, Jagan Teki wrote:
> > TM2 board DSI pipeline has input from MIC and output to
> > s6e3ha2 panel.
> >
> > The existing pipeline has child nodes of ports, panel and
> > MIC is remote-endpoint reference of port@0 of ports.
> >
> > Adding panel as another child node to DSI is unconventional
> > as pipeline has ports child. However it can be true if MIC
> > is added inside port node like this.
> >
> > dsi {
> > compatible = "samsung,exynos5433-mipi-dsi";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > port {
> > dsi_to_mic: endpoint {
> > remote-endpoint = <&mic_to_dsi>;
> > };
> > };
> >
> > panel@0 {
> > compatible = "samsung,s6e3hf2";
> > reg = <0>;
> > vdd3-supply = <&ldo27_reg>;
> > vci-supply = <&ldo28_reg>;
> > reset-gpios = <&gpg0 0 GPIO_ACTIVE_LOW>;
> > enable-gpios = <&gpf1 5 GPIO_ACTIVE_HIGH>;
> > };
> > };
> >
> > The above pipeline is proper but it requires the DSI input MIC
> > pipeline to update.
> >
> > This patch is trying to add panel at port@1 so-that the entire
> > pipeline before to panel output is untouched.
> >
> > Reported-by: Marek Szyprowski
> > Signed-off-by: Jagan Teki
> > ---
> > arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 16
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> > b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> > index aca01709fd29..e13210c8d7e0 100644
> > --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> > +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> > @@ -53,6 +53,16 @@ &cmu_disp {
> > };
> >
> > &dsi {
> > + ports {
> > + port@1 {
> > + reg = <1>;
> > +
> > + dsi_out_panel: endpoint {
> > + remote-endpoint = <&dsi_in_panel>;
> > + };
> > + };
> > + };
> > +
> > panel@0 {
> > compatible = "samsung,s6e3ha2";
> > reg = <0>;
> > @@ -60,6 +70,12 @@ panel@0 {
> > vci-supply = <&ldo28_reg>;
> > reset-gpios = <&gpg0 0 GPIO_ACTIVE_LOW>;
> > enable-gpios = <&gpf1 5 GPIO_ACTIVE_HIGH>;
> > +
> > + port {
> > + dsi_in_panel: endpoint {
> > + remote-endpoint = <&dsi_out_panel>;
> > + };
> > + };
>
>
> As I already wrote in Exynos thread, DSI host has already parent-child
> relation with the panel - DSI host knows well who is connected to it.
> Adding another links between them is redundant and has no value added.
>
> I have already answered in Exynos thread[1] how could you deal with the
> issue, you have.
>
> [1]:
> https://lore.kernel.org/dri-devel/e541c52b-9751-933b-5eac-783dd0ed9...@intel.com/
I have commented on this thread for better understanding. Please have
a look and respond.
Thanks,
Jagan.
Re: [PATCH RESEND v4 v5 4/4] drm/vc4: Notify the firmware when DRM is in charge
Hi
Am 15.12.21 um 10:51 schrieb Maxime Ripard:
Once the call to drm_fb_helper_remove_conflicting_framebuffers() has
been made, simplefb has been unregistered and the KMS driver is entirely
in charge of the display.
Thus, we can notify the firmware it can free whatever resource it was
using to maintain simplefb functional.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_drv.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 86c61ee120b7..a03053c8e22c 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -37,6 +37,8 @@
#include
#include
+#include
+
#include "uapi/drm/vc4_drm.h"
#include "vc4_drv.h"
@@ -215,6 +217,7 @@ static void vc4_match_add_drivers(struct device *dev,
static int vc4_drm_bind(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
+ struct rpi_firmware *firmware = NULL;
struct drm_device *drm;
struct vc4_dev *vc4;
struct device_node *node;
@@ -251,10 +254,29 @@ static int vc4_drm_bind(struct device *dev)
if (ret)
return ret;
+ node = of_find_compatible_node(NULL, NULL, "raspberrypi,bcm2835-firmware");
+ if (node) {
+ firmware = rpi_firmware_get(node);
+ of_node_put(node);
+
+ if (!firmware)
+ return -EPROBE_DEFER;
+ }
+
The code is
Acked-by: Thomas Zimmermann
Just for my understanding:
You retrieve the firmware before killing simpledrm simply to keep the
display on if it fails, right?
What's the possible error that would justify a retry (via EPROBE_DEFER)?
Best regards
Thomas
ret = drm_aperture_remove_framebuffers(false, &vc4_drm_driver);
if (ret)
return ret;
+ if (firmware) {
+ ret = rpi_firmware_property(firmware,
+ RPI_FIRMWARE_NOTIFY_DISPLAY_DONE,
+ NULL, 0);
+ if (ret)
+ drm_warn(drm, "Couldn't stop firmware display driver:
%d\n", ret);
+
+ rpi_firmware_put(firmware);
+ }
+
ret = component_bind_all(dev, drm);
if (ret)
return ret;
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: (subset) [PATCH] drm/sun4i: dw-hdmi: Fix missing put_device() call in sun8i_hdmi_phy_get
On Fri, 7 Jan 2022 08:36:32 +, Miaoqian Lin wrote: > The reference taken by 'of_find_device_by_node()' must be released when > not needed anymore. > Add the corresponding 'put_device()' in the error handling path. > > Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
[Bug 215001] Regression in 5.15, Firmware-initialized graphics console selects FB_VGA16, screen corruption
https://bugzilla.kernel.org/show_bug.cgi?id=215001 --- Comment #12 from Javier Martinez Canillas (jav...@dowhile0.org) --- Hello Kris, Could you please also test the patches in v2? https://lore.kernel.org/dri-devel/20220110095625.278836-1-javi...@redhat.com/ There shouldn't be any changes with respect to v1, but just wanted to be safe before pushing to drm-misc since I can't test them. Thanks a lot and best regards, Javier -- You may reply to this email to add a comment. You are receiving this mail because: You are on the CC list for the bug.
[PATCH 1/3] drm: add writeback pointers to drm_connector
Changing drm_connector and drm_encoder feilds to pointers in
drm_writeback_connector as the elements of struct
drm_writeback_connector are:
struct drm_writeback_connector {
struct drm_connector base;
struct drm_encoder encoder;
Similarly the elements of intel_encoder and intel_connector
are:
struct intel_encoder {
struct drm_encoder base;
struct intel_connector {
struct drm_connector base;
The function drm_writeback_connector_init() will initialize the
drm_connector and drm_encoder and attach them as well.
Since the drm_connector/encoder are both struct in
drm_writeback_connector and intel_connector/encoder, we need
one of them to be a pointer so we can reference them or else we
will be pointing to 2 seprate instances.
Usually the struct defined in drm framework pointing to any
struct will be pointer and allocating them and initialization
will be done with the users.
Like struct drm_connector and drm_encoder are part of drm
framework and the users of these such as i915 have included them
in their struct intel_connector and intel_encoder. Likewise
struct drm_writeback_connector is a special connector and hence
is not a user of drm_connector and hence this should be pointers.
Adding drm_writeback_connector to drm_connector so that
writeback_connector can be fetched from drm_connector as the previous
container_of method won't work due to change in the feilds of
drm_connector and drm_encoder in drm_writeback_connector.
Note:The corresponding ripple effect due to the above changes namely in
two drivers as I can see it komeda and vkms have been dealt with in the
upcoming patches of this series.
Signed-off-by: Kandpal, Suraj
---
drivers/gpu/drm/drm_writeback.c | 19 ++-
include/drm/drm_connector.h | 3 +++
include/drm/drm_writeback.h | 6 +++---
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index dccf4504f1bb..47238db42363 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -87,7 +87,7 @@ static const char *drm_writeback_fence_get_driver_name(struct
dma_fence *fence)
struct drm_writeback_connector *wb_connector =
fence_to_wb_connector(fence);
- return wb_connector->base.dev->driver->name;
+ return wb_connector->base->dev->driver->name;
}
static const char *
@@ -177,7 +177,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
const u32 *formats, int n_formats)
{
struct drm_property_blob *blob;
- struct drm_connector *connector = &wb_connector->base;
+ struct drm_connector *connector = wb_connector->base;
struct drm_mode_config *config = &dev->mode_config;
int ret = create_writeback_properties(dev);
@@ -189,14 +189,15 @@ int drm_writeback_connector_init(struct drm_device *dev,
if (IS_ERR(blob))
return PTR_ERR(blob);
- drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
- ret = drm_encoder_init(dev, &wb_connector->encoder,
+ drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
+ ret = drm_encoder_init(dev, wb_connector->encoder,
&drm_writeback_encoder_funcs,
DRM_MODE_ENCODER_VIRTUAL, NULL);
if (ret)
goto fail;
connector->interlace_allowed = 0;
+ connector->wb_connector = wb_connector;
ret = drm_connector_init(dev, connector, con_funcs,
DRM_MODE_CONNECTOR_WRITEBACK);
@@ -204,7 +205,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
goto connector_fail;
ret = drm_connector_attach_encoder(connector,
- &wb_connector->encoder);
+ wb_connector->encoder);
if (ret)
goto attach_fail;
@@ -233,7 +234,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
attach_fail:
drm_connector_cleanup(connector);
connector_fail:
- drm_encoder_cleanup(&wb_connector->encoder);
+ drm_encoder_cleanup(wb_connector->encoder);
fail:
drm_property_blob_put(blob);
return ret;
@@ -263,7 +264,7 @@ int drm_writeback_prepare_job(struct drm_writeback_job *job)
{
struct drm_writeback_connector *connector = job->connector;
const struct drm_connector_helper_funcs *funcs =
- connector->base.helper_private;
+ connector->base->helper_private;
int ret;
if (funcs->prepare_writeback_job) {
@@ -315,7 +316,7 @@ void drm_writeback_cleanup_job(struct drm_writeback_job
*job)
{
struct drm_writeback_connector *connector = job->connector;
const struct drm_connector_helper_funcs *funcs =
- connector->base.helper_private;
+ connector->base->helper_pr
[PATCH 3/3] drm/vkms: change vkms driver to use drm_writeback_connector.base pointer
Changing vkms driver to accomadate the change of
drm_writeback_connector.base to a pointer the reason for which is
explained in the Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal, Suraj
---
drivers/gpu/drm/vkms/vkms_writeback.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c
b/drivers/gpu/drm/vkms/vkms_writeback.c
index 8694227f555f..6de0c4213425 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -114,7 +114,7 @@ static void vkms_wb_atomic_commit(struct drm_connector
*conn,
struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev);
struct vkms_output *output = &vkmsdev->output;
struct drm_writeback_connector *wb_conn = &output->wb_connector;
- struct drm_connector_state *conn_state = wb_conn->base.state;
+ struct drm_connector_state *conn_state = wb_conn->base->state;
struct vkms_crtc_state *crtc_state = output->composer_state;
if (!conn_state)
@@ -140,8 +140,8 @@ int vkms_enable_writeback_connector(struct vkms_device
*vkmsdev)
{
struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;
- vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;
- drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
+ vkmsdev->output.wb_connector.encoder->possible_crtcs = 1;
+ drm_connector_helper_add(wb->base, &vkms_wb_conn_helper_funcs);
return drm_writeback_connector_init(&vkmsdev->drm, wb,
&vkms_wb_connector_funcs,
--
2.17.1
[PATCH 2/3] drm/arm/komeda : change driver to use drm_writeback_connector.base pointer
Making changes to komeda driver because we had to change
drm_writeback_connector.base into a pointer the reason for which is
expained in the Patch (drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal, Suraj
---
drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 +-
drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 3 ++-
drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c | 9 +
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 59172acb9738..eb37f41c1790 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -265,7 +265,7 @@ komeda_crtc_do_flush(struct drm_crtc *crtc,
if (slave && has_bit(slave->id, kcrtc_st->affected_pipes))
komeda_pipeline_update(slave, old->state);
- conn_st = wb_conn ? wb_conn->base.base.state : NULL;
+ conn_st = wb_conn ? wb_conn->base.base->state : NULL;
if (conn_st && conn_st->writeback_job)
drm_writeback_queue_job(&wb_conn->base, conn_st);
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index 456f3c435719..8d83883a1d99 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -53,6 +53,7 @@ struct komeda_plane_state {
* struct komeda_wb_connector
*/
struct komeda_wb_connector {
+ struct drm_connector conn;
/** @base: &drm_writeback_connector */
struct drm_writeback_connector base;
@@ -136,7 +137,7 @@ struct komeda_kms_dev {
static inline bool is_writeback_only(struct drm_crtc_state *st)
{
struct komeda_wb_connector *wb_conn = to_kcrtc(st->crtc)->wb_conn;
- struct drm_connector *conn = wb_conn ? &wb_conn->base.base : NULL;
+ struct drm_connector *conn = wb_conn ? wb_conn->base.base : NULL;
return conn && (st->connector_mask == BIT(drm_connector_index(conn)));
}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index e465cc4879c9..0caaf483276d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -51,7 +51,7 @@ komeda_wb_encoder_atomic_check(struct drm_encoder *encoder,
return -EINVAL;
}
- wb_layer = to_kconn(to_wb_conn(conn_st->connector))->wb_layer;
+ wb_layer =
to_kconn(drm_connector_to_writeback(conn_st->connector))->wb_layer;
/*
* No need for a full modested when the only connector changed is the
@@ -123,7 +123,7 @@ komeda_wb_connector_fill_modes(struct drm_connector
*connector,
static void komeda_wb_connector_destroy(struct drm_connector *connector)
{
drm_connector_cleanup(connector);
- kfree(to_kconn(to_wb_conn(connector)));
+ kfree(to_kconn(drm_connector_to_writeback(connector)));
}
static const struct drm_connector_funcs komeda_wb_connector_funcs = {
@@ -155,6 +155,7 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
kwb_conn->wb_layer = kcrtc->master->wb_layer;
wb_conn = &kwb_conn->base;
+ wb_conn->base = &kwb_conn->conn;
wb_conn->encoder.possible_crtcs = BIT(drm_crtc_index(&kcrtc->base));
formats = komeda_get_layer_fourcc_list(&mdev->fmt_tbl,
@@ -171,9 +172,9 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
return err;
}
- drm_connector_helper_add(&wb_conn->base, &komeda_wb_conn_helper_funcs);
+ drm_connector_helper_add(wb_conn->base, &komeda_wb_conn_helper_funcs);
- info = &kwb_conn->base.base.display_info;
+ info = &kwb_conn->base.base->display_info;
info->bpc = __fls(kcrtc->master->improc->supported_color_depths);
info->color_formats = kcrtc->master->improc->supported_color_formats;
--
2.17.1
Re: [PATCH v2 3/5] drm/dp: Move DisplayPort helpers into separate helper module
On Wed, Dec 15, 2021 at 12:12 PM Thomas Zimmermann wrote: > > Hi > > Am 15.12.21 um 12:04 schrieb Jani Nikula: > > On Wed, 15 Dec 2021, Thomas Zimmermann wrote: > >> * move DP helper code into dp/ (Jani) > > > > I suggested adding the subdirectory, but I'm going to bikeshed the name, > > which I didn't suggest. > > > > $ find drivers/gpu/drm -mindepth 1 -maxdepth 1 -type d | wc -l > > 68 > > > > Assuming we move more of the drm modules to subdirectories, how are they > > going to stand out from drivers? > > > > I suggested drm_dp, which I understand results in tautology, but hey, > > all the filenames under drm/ also have drm_*.[ch]. And I find that very > > useful for git greps and other code archeology. With just the dp name, > > you'd have to know and list all the drm subdirectories when looking up > > stuff that's part of drm but not drivers. > > I think we have enough filename prefixes already. drm/drm_dp/drm_dp_ is > just ridiculous. > Maybe what can be done is to just add a drivers/gpu/drm/core subdirectory that would contain all the DRM core code ? Then the dp helpers could be moved to drivers/gpu/drm/core/dp/drm_dp.c for example. This would also make easy to differentiate the drm modules from the drivers with just: $ find drivers/gpu/drm -mindepth 1 -maxdepth 1 -type d -not -name core Best regards, Javier
[Bug 215445] AMDGPU -- UBSAN: invalid-load in amdgpu_dm.c:5882:84 - load of value 32 is not a valid value for type '_Bool'
https://bugzilla.kernel.org/show_bug.cgi?id=215445 --- Comment #5 from Martin Pecka (pe...@seznam.cz) --- This problem has disappeared for me in kernel 5.16.0. Can anybody else confirm? -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH] i915: make array flex_regs static const
On 11/01/2022 09:13, Tvrtko Ursulin wrote:
On 09/01/2022 20:31, Colin Ian King wrote:
Don't populate the read-only array flex_regs on the stack but
instead it static const. Also makes the object code a little smaller.
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/i915/i915_perf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c
b/drivers/gpu/drm/i915/i915_perf.c
index e27f3b7cf094..df698960fdc0 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2114,7 +2114,7 @@ gen8_update_reg_state_unlocked(const struct
intel_context *ce,
u32 ctx_oactxctrl = stream->perf->ctx_oactxctrl_offset;
u32 ctx_flexeu0 = stream->perf->ctx_flexeu0_offset;
/* The MMIO offsets for Flex EU registers aren't contiguous */
- i915_reg_t flex_regs[] = {
+ static const i915_reg_t flex_regs[] = {
EU_PERF_CNTL0,
EU_PERF_CNTL1,
EU_PERF_CNTL2,
Reviewed-by: Tvrtko Ursulin
And will merge shortly, thanks for the patch.
Actually I couldn't merge it because you have a Author and Signed-off-by
mismatch due your entry in .mailmap. Is this something you can update or
send the patch from an address which matches it?
Regards,
Tvrtko
Re: [PATCH 3/5] dt-bindings: display: simple: add Geekworm MZP280 Panel
Hi, On Mon, Jan 03, 2022 at 11:41:04AM -0600, Chris Morgan wrote: > From: Chris Morgan > > The Geekworm MZP280 panel is a 480x640 (portrait) panel with a > capacitive touch interface and a 40 pin header meant to interface > directly with the Raspberry Pi. The screen is 2.8 inches diagonally, > and there appear to be at least 4 distinct versions all with the same > panel timings. > > Timings were derived from drivers posted on the github located here: > https://github.com/tianyoujian/MZDPI/tree/master/vga > > Additional details about this panel family can be found here: > https://wiki.geekworm.com/2.8_inch_Touch_Screen_for_Pi_zero > > Signed-off-by: Chris Morgan > --- > .../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 f3c9395d23b6..659db7206c96 100644 > --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > @@ -148,6 +148,8 @@ properties: >- frida,frd350h54004 > # FriendlyELEC HD702E 800x1280 LCD panel >- friendlyarm,hd702e > +# Geekworm MZP280 2.8" 480x640 LCD panel with capacitive touch > + - geekworm,mzp280 The vendor prefix must be documented in Documentation/devicetree/bindings/vendor-prefixes.yaml Maxime signature.asc Description: PGP signature
Re: [PATCH 4/5] drm/panel: simple: add Geekworm MZP280 Panel
On Mon, Jan 03, 2022 at 11:41:05AM -0600, Chris Morgan wrote: > From: Chris Morgan > > Add support for the Geekworm MZP280 Panel > > Signed-off-by: Chris Morgan Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH v9 13/15] memory: mtk-smi: Get rid of mtk_smi_larb_get/put
Il 12/11/21 11:55, Yong Wu ha scritto: After adding device_link between the iommu consumer and smi-larb, the pm_runtime_get(_sync) of smi-larb and smi-common will be called automatically. we can get rid of mtk_smi_larb_get/put. CC: Matthias Brugger Signed-off-by: Yong Wu Reviewed-by: Evan Green Acked-by: Krzysztof Kozlowski Acked-by: Matthias Brugger Reviewed-by: Dafna Hirschfeld Tested-by: Frank Wunderlich # BPI-R2/MT7623 Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 15/15] arm64: dts: mediatek: Get rid of mediatek, larb for MM nodes
Il 12/11/21 11:55, Yong Wu ha scritto: After adding device_link between the IOMMU consumer and smi, the mediatek,larb is unnecessary now. CC: Matthias Brugger Signed-off-by: Yong Wu Reviewed-by: Evan Green Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 12/15] media: mtk-vcodec: enc: Remove mtk_vcodec_release_enc_pm
Il 12/11/21 11:55, Yong Wu ha scritto: After this patchset, mtk_vcodec_release_enc_pm has only one line. then remove that function, use pm_runtime_disable instead. meanwhile, mtk_vcodec_init_enc_pm only operate for the clocks, rename it from the _pm to _clk. No functional change. CC: Tiffany Lin CC: Irui Wang Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 10/15] media: mtk-vcodec: Get rid of mtk_smi_larb_get/put
Il 12/11/21 11:55, Yong Wu ha scritto: MediaTek IOMMU has already added the device_link between the consumer and smi-larb device. If the vcodec device call the pm_runtime_get_sync, the smi-larb's pm_runtime_get_sync also be called automatically. CC: Tiffany Lin CC: Irui Wang Signed-off-by: Yong Wu Reviewed-by: Evan Green Acked-by: Tiffany Lin Reviewed-by: Dafna Hirschfeld Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 11/15] media: mtk-vcodec: dec: Remove mtk_vcodec_release_dec_pm
Il 12/11/21 11:55, Yong Wu ha scritto: After this patchset, mtk_vcodec_release_dec_pm has only one line. then remove that function. Use pm_runtime_disable directly instead. For symmetry, move the pm_runtime_enable out from mtk_vcodec_init_dec_pm, then mtk_vcodec_init_dec_pm only operate for the clocks, rename it from the _pm to _clk. No functional change. CC: Tiffany Lin CC: Yunfei Dong Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 08/15] drm/mediatek: Add pm runtime support for ovl and rdma
Il 12/11/21 11:55, Yong Wu ha scritto: From: Yongqiang Niu Prepare for smi cleaning up "mediatek,larb". Display use the dispsys device to call pm_rumtime_get_sync before. This patch add pm_runtime_xx with ovl and rdma device whose nodes has "iommus" property, then display could help pm_runtime_get for smi via ovl or rdma device. CC: CK Hu Signed-off-by: Yongqiang Niu Signed-off-by: Yong Wu (Yong: Use pm_runtime_resume_and_get instead of pm_runtime_get_sync) Acked-by: Chun-Kuang Hu Tested-by: Frank Wunderlich # BPI-R2/MT7623 Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 06/15] media: mtk-jpeg: Get rid of mtk_smi_larb_get/put
Il 12/11/21 11:55, Yong Wu ha scritto: MediaTek IOMMU has already added device_link between the consumer and smi-larb device. If the jpg device call the pm_runtime_get_sync, the smi-larb's pm_runtime_get_sync also be called automatically. After removing the larb_get operations, then mtk_jpeg_clk_init is also unnecessary. Remove it too. CC: Rick Chang CC: Xia Jiang Signed-off-by: Yong Wu Reviewed-by: Evan Green Acked-by: Rick Chang Reviewed-by: Dafna Hirschfeld Tested-by: Frank Wunderlich # BPI-R2/MT7623 Acked-by: AngeloGioacchino Del Regno
Re: [PATCH v9 07/15] media: mtk-mdp: Get rid of mtk_smi_larb_get/put
Il 12/11/21 11:55, Yong Wu ha scritto: MediaTek IOMMU has already added the device_link between the consumer and smi-larb device. If the mdp device call the pm_runtime_get_sync, the smi-larb's pm_runtime_get_sync also be called automatically. CC: Minghsiu Tsai CC: Houlong Wei Signed-off-by: Yong Wu Reviewed-by: Evan Green Reviewed-by: Houlong Wei Reviewed-by: Dafna Hirschfeld Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 09/15] drm/mediatek: Get rid of mtk_smi_larb_get/put
Il 12/11/21 11:55, Yong Wu ha scritto: MediaTek IOMMU has already added the device_link between the consumer and smi-larb device. If the drm device call the pm_runtime_get_sync, the smi-larb's pm_runtime_get_sync also be called automatically. CC: CK Hu CC: Philipp Zabel Signed-off-by: Yong Wu Reviewed-by: Evan Green Acked-by: Chun-Kuang Hu Reviewed-by: Dafna Hirschfeld Tested-by: Frank Wunderlich # BPI-R2/MT7623 Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 03/15] iommu/mediatek: Return ENODEV if the device is NULL
Il 12/11/21 11:54, Yong Wu ha scritto: The platform device is created at: of_platform_default_populate_init: arch_initcall_sync ->of_platform_populate ->of_platform_device_create_pdata When entering our probe, all the devices should be already created. if it is null, means NODEV. Currently we don't get the fail case. It's a minor fix, no need add fixes tags. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 01/15] dt-binding: mediatek: Get rid of mediatek, larb for multimedia HW
Il 12/11/21 11:54, Yong Wu ha scritto: After adding device_link between the consumer with the smi-larbs, if the consumer call its owner pm_runtime_get(_sync), the pm_runtime_get(_sync) of smi-larb and smi-common will be called automatically. Thus, the consumer don't need this property. And IOMMU also know which larb this consumer connects with from iommu id in the "iommus=" property. Signed-off-by: Yong Wu Reviewed-by: Rob Herring Reviewed-by: Evan Green Acked-by: AngeloGioacchino Del Regno
Re: [PATCH v9 04/15] iommu/mediatek: Add probe_defer for smi-larb
Il 12/11/21 11:54, Yong Wu ha scritto: Prepare for adding device_link. The iommu consumer should use device_link to connect with the smi-larb(supplier). then the smi-larb should run before the iommu consumer. Here we delay the iommu driver until the smi driver is ready, then all the iommu consumers always are after the smi driver. When there is no this patch, if some consumer drivers run before smi-larb, the supplier link_status is DL_DEV_NO_DRIVER(0) in the device_link_add, then device_links_driver_bound will use WARN_ON to complain that the link_status of supplier is not right. device_is_bound may be more elegant here. but it is not allowed to EXPORT from https://lore.kernel.org/patchwork/patch/1334670/. Signed-off-by: Yong Wu Tested-by: Frank Wunderlich # BPI-R2/MT7623 Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 05/15] iommu/mediatek: Add device_link between the consumer and the larb devices
Il 12/11/21 11:54, Yong Wu ha scritto: MediaTek IOMMU-SMI diagram is like below. all the consumer connect with smi-larb, then connect with smi-common. M4U | smi-common | - | |... | | larb1 larb2 | | vdec venc When the consumer works, it should enable the smi-larb's power which also need enable the smi-common's power firstly. Thus, First of all, use the device link connect the consumer and the smi-larbs. then add device link between the smi-larb and smi-common. This patch adds device_link between the consumer and the larbs. When device_link_add, I add the flag DL_FLAG_STATELESS to avoid calling pm_runtime_xx to keep the original status of clocks. It can avoid two issues: 1) Display HW show fastlogo abnormally reported in [1]. At the beggining, all the clocks are enabled before entering kernel, but the clocks for display HW(always in larb0) will be gated after clk_enable and clk_disable called from device_link_add(->pm_runtime_resume) and rpm_idle. The clock operation happened before display driver probe. At that time, the display HW will be abnormal. 2) A deadlock issue reported in [2]. Use DL_FLAG_STATELESS to skip pm_runtime_xx to avoid the deadlock. Corresponding, DL_FLAG_AUTOREMOVE_CONSUMER can't be added, then device_link_removed should be added explicitly. Meanwhile, Currently we don't have a device connect with 2 larbs at the same time. Disallow this case, print the error log. [1] https://lore.kernel.org/linux-mediatek/1564213888.22908.4.camel@mhfsdcap03/ [2] https://lore.kernel.org/patchwork/patch/1086569/ Suggested-by: Tomasz Figa Signed-off-by: Yong Wu Tested-by: Frank Wunderlich # BPI-R2/MT7623 Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v9 02/15] iommu/mediatek-v1: Free the existed fwspec if the master dev already has
Il 12/11/21 11:54, Yong Wu ha scritto:
When the iommu master device enters of_iommu_xlate, the ops may be
NULL(iommu dev is defered), then it will initialize the fwspec here:
[] (dev_iommu_fwspec_set) from []
(iommu_fwspec_init+0xbc/0xd4)
[] (iommu_fwspec_init) from []
(of_iommu_xlate+0x7c/0x12c)
[] (of_iommu_xlate) from []
(of_iommu_configure+0x144/0x1e8)
BUT the mtk_iommu_v1.c only supports arm32, the probing flow still is a bit
weird. We always expect create the fwspec internally. otherwise it will
enter here and return fail.
static int mtk_iommu_create_mapping(struct device *dev,
struct of_phandle_args *args)
{
...
if (!fwspec) {
} else if (dev_iommu_fwspec_get(dev)->ops != &mtk_iommu_ops) {
>>Enter here. return fail.
return -EINVAL;
}
...
}
Thus, Free the existed fwspec if the master device already has fwspec.
This issue is reported at:
https://lore.kernel.org/linux-mediatek/trinity-7d9ebdc9-4849-4d93-bfb5-429dcb4ee449-1626253158870@3c-app-gmx-bs01/
Reported-by: Frank Wunderlich
Tested-by: Frank Wunderlich # BPI-R2/MT7623
Signed-off-by: Yong Wu
Acked-by: AngeloGioacchino Del Regno
Re: [PATCH] dma-buf: Move sysfs work out of DMA-BUF export/release path
Am 11.01.22 um 09:35 schrieb Daniel Vetter: On Tue, Jan 11, 2022 at 7:02 AM Hridya Valsaraju wrote: On Sun, Jan 9, 2022 at 11:28 PM Christian König wrote: Am 07.01.22 um 22:25 schrieb Hridya Valsaraju: On Fri, Jan 7, 2022 at 10:17 AM Hridya Valsaraju wrote: On Fri, Jan 7, 2022 at 2:22 AM Christian König wrote: Am 06.01.22 um 20:04 schrieb Hridya Valsaraju: On Thu, Jan 6, 2022 at 12:59 AM Christian König wrote: Am 05.01.22 um 00:51 schrieb Hridya Valsaraju: Recently, we noticed an issue where a process went into direct reclaim while holding the kernfs rw semaphore for sysfs in write(exclusive) mode. This caused processes who were doing DMA-BUF exports and releases to go into uninterruptible sleep since they needed to acquire the same semaphore for the DMA-BUF sysfs entry creation/deletion. In order to avoid blocking DMA-BUF export/release for an indeterminate amount of time while another process is holding the sysfs rw semaphore in exclusive mode, this patch moves the per-buffer sysfs file creation/deleteion to a kthread. Well I absolutely don't think that this is justified. You adding tons of complexity here just to avoid the overhead of creating the sysfs files while exporting DMA-bufs which is an operation which should be done exactly once in the lifecycle for the most common use case. Please explain further why that should be necessary. Hi Christian, We noticed that the issue sometimes causes the exporting process to go to the uninterrupted sleep state for tens of milliseconds which unfortunately leads to user-perceptible UI jank. This is the reason why we are trying to move the sysfs entry creation and deletion out of the DMA-BUF export/release path. I will update the commit message to include the same in the next revision. That is still not a justification for this change. The question is why do you need that in the first place? Exporting a DMA-buf should be something would should be very rarely, e.g. only at the start of an application. Hi Christian, Yes, that is correct. Noticeable jank caused by the issue is not present at all times and happens on UI transitions(for example during app launches and exits) when there are several DMA-BUFs being exported and released. This is especially true in the case of the camera app since it exports and releases a relatively larger number of DMA-BUFs during launch and exit respectively. Well, that sounds at least better than before. Regards, Hridya So this strongly looks like you are trying to optimize for an use case where we should probably rethink what userspace is doing here instead. Hello Christian, If you don't mind, could you please elaborate on the above statement? The purpose of DMA-buf is to share a rather low number of buffers between different drivers and/or applications. For example with triple buffering we have three buffers shared between the camera driver and the display driver, same thing for use cases between rendering and display. So even when you have ~100 applications open your should not share more than ~300 DMA-buf handles and doing that should absolutely not cause any problems like you described above. Long story short when this affects your user experience then your user space is exporting *much* more buffers than expected. Especially since the sysfs overhead is completely negligible compared to the overhead drivers have when they export buffers. I do not think we can solve this issue from userspace since the problem is not due to the overhead of sysfs creation/teardown itself. Yes, thanks. That's the important information I was looking for. The problem is that the duration of time for which the exporting process would need to sleep waiting for the kernfs_rwsem semaphore is undefined when the holder of the semaphore goes under direct reclaim. Fsnotify events for sysfs files are also generated while holding the same semaphore. This is also not a problem due to the high number of DMA-BUF exports during launch time, as even a single export can be delayed for an unpredictable amount of time. We cannot eliminate DMA-BUF exports completely during app-launches and we are unfortunately seeing reports of the exporting process occasionally sleeping long enough to cause user-visible jankiness :( We also looked at whether any optimizations are possible from the kernfs implementation side[1] but the semaphore is used quite extensively and it looks like the best way forward would be to remove sysfs creation/teardown from the DMA-BUF export/release path altogether. We have some ideas on how we can reduce the code-complexity in the current patch. If we manage to simplify it considerably, would the approach of offloading sysfs creation and teardown into a separate thread be acceptable Christian? At bare minimum I suggest to use a work_struct instead of re-inventing that with kthread. And then only put the exporting of buffers into the background and not the teardown. Thank you for the guidance! One worr
Re: [PATCH v2] drm/mediatek: mtk_dsi: Avoid EPROBE_DEFER loop with external bridge
Il 06/01/22 06:22, CK Hu ha scritto: Hi, Angelo: On Tue, 2022-01-04 at 10:59 +0100, AngeloGioacchino Del Regno wrote: DRM bridge drivers are now attaching their DSI device at probe time, which requires us to register our DSI host in order to let the bridge to probe: this recently started producing an endless -EPROBE_DEFER loop on some machines that are using external bridges, like the parade-ps8640, found on the ACER Chromebook R13. Now that the DSI hosts/devices probe sequence is documented, we can do adjustments to the mtk_dsi driver as to both fix now and make sure to avoid this situation in the future: for this, following what is documented in drm_bridge.c, move the mtk_dsi component_add() to the mtk_dsi_ops.attach callback and delete it in the detach callback; keeping in mind that we are registering a drm_bridge for our DSI, which is only used/attached if the DSI Host is bound, it wouldn't make sense to keep adding our bridge at probe time (as it would be useless to have it if mtk_dsi_ops.attach() fails!), so also move that one to the dsi host attach function (and remove it in detach). This patch looks good to me, but please add 'Fixes' tag to note to which patch this patch want to fix. Regards, CK Hello, unfortunately I couldn't find a valid commit for a Fixes tag. This started being an issue at some point, when the DRM API was changed to adhere to the documented probe sequence: the MediaTek DSI driver was not the only one that got broken/affected by these changes. If you have any advice on which commit should be tagged, I'm open for any kind of suggestion. Thanks, - Angelo
[PATCH v3 1/2] dt-bindings: display: Turn lvds.yaml into a generic schema
The lvds.yaml file so far was both defining the generic LVDS properties
(such as data-mapping) that could be used for any LVDS sink, but also
the panel-lvds binding.
That last binding was to describe LVDS panels simple enough, and had a
number of other bindings using it as a base to specialise it further.
However, this situation makes it fairly hard to extend and reuse both
the generic parts, and the panel-lvds itself.
Let's remove the panel-lvds parts and leave only the generic LVDS
properties.
Reviewed-by: Rob Herring
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Fix references to that file
Changes from v1:
- Moved the schema out of panel
---
.../bindings/display/bridge/lvds-codec.yaml | 2 +-
.../bindings/display/{panel => }/lvds.yaml| 31 ++-
.../display/panel/advantech,idk-1110wr.yaml | 19 ++--
.../display/panel/innolux,ee101ia-01d.yaml| 23 --
.../display/panel/mitsubishi,aa104xd12.yaml | 19 ++--
.../display/panel/mitsubishi,aa121td01.yaml | 19 ++--
.../display/panel/sgd,gktw70sdae4se.yaml | 19 ++--
MAINTAINERS | 2 +-
8 files changed, 93 insertions(+), 41 deletions(-)
rename Documentation/devicetree/bindings/display/{panel => }/lvds.yaml (86%)
diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
index 5079c1cc337b..27b905b81b12 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
@@ -67,7 +67,7 @@ properties:
- vesa-24
description: |
The color signals mapping order. See details in
- Documentation/devicetree/bindings/display/panel/lvds.yaml
+ Documentation/devicetree/bindings/display/lvds.yaml
port@1:
$ref: /schemas/graph.yaml#/properties/port
diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml
b/Documentation/devicetree/bindings/display/lvds.yaml
similarity index 86%
rename from Documentation/devicetree/bindings/display/panel/lvds.yaml
rename to Documentation/devicetree/bindings/display/lvds.yaml
index 49460c9dceea..55751402fb13 100644
--- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
+++ b/Documentation/devicetree/bindings/display/lvds.yaml
@@ -1,10 +1,10 @@
# SPDX-License-Identifier: GPL-2.0
%YAML 1.2
---
-$id: http://devicetree.org/schemas/display/panel/lvds.yaml#
+$id: http://devicetree.org/schemas/display/lvds.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: LVDS Display Panel
+title: LVDS Display Common Properties
maintainers:
- Laurent Pinchart
@@ -26,18 +26,7 @@ description: |+
Device compatible with those specifications have been marketed under the
FPD-Link and FlatLink brands.
-allOf:
- - $ref: panel-common.yaml#
-
properties:
- compatible:
-contains:
- const: panel-lvds
-description:
- Shall contain "panel-lvds" in addition to a mandatory panel-specific
- compatible string defined in individual panel bindings. The "panel-lvds"
- value shall never be used on its own.
-
data-mapping:
enum:
- jeida-18
@@ -96,22 +85,6 @@ properties:
If set, reverse the bit order described in the data mappings below on all
data lanes, transmitting bits for slots 6 to 0 instead of 0 to 6.
- port: true
- ports: true
-
-required:
- - compatible
- - data-mapping
- - width-mm
- - height-mm
- - panel-timing
-
-oneOf:
- - required:
- - port
- - required:
- - ports
-
additionalProperties: true
...
diff --git
a/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml
b/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml
index 93878c2cd370..3a8c2c11f9bd 100644
--- a/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml
+++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml
@@ -11,13 +11,23 @@ maintainers:
- Thierry Reding
allOf:
- - $ref: lvds.yaml#
+ - $ref: panel-common.yaml#
+ - $ref: /schemas/display/lvds.yaml/#
+
+select:
+ properties:
+compatible:
+ contains:
+const: advantech,idk-1110wr
+
+ required:
+- compatible
properties:
compatible:
items:
- const: advantech,idk-1110wr
- - {} # panel-lvds, but not listed here to avoid false select
+ - const: panel-lvds
data-mapping:
const: jeida-24
@@ -35,6 +45,11 @@ additionalProperties: false
required:
- compatible
+ - data-mapping
+ - width-mm
+ - height-mm
+ - panel-timing
+ - port
examples:
- |+
diff --git
a/Documentation/devicetree/bindings/display/panel/innolux,ee101ia-01d.yaml
b/Documentation/devicetree/bindings/display/panel/innolux,ee101ia-01d.yaml
index a69681e724cb..566e11f6bfc0 100644
--- a/Documentati
[PATCH v3 2/2] dt-bindings: panel: Introduce a panel-lvds binding
Following the previous patch, let's introduce a generic panel-lvds binding that documents the panels that don't have any particular constraint documented. Reviewed-by: Rob Herring Signed-off-by: Maxime Ripard --- Changes from v2: - Added a MAINTAINERS entry Changes from v1: - Added missing compatible - Fixed lint --- .../bindings/display/panel/panel-lvds.yaml| 57 +++ MAINTAINERS | 1 + 2 files changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/panel-lvds.yaml diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml new file mode 100644 index ..fcc50db6a812 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/panel-lvds.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Generic LVDS Display Panel Device Tree Bindings + +maintainers: + - Lad Prabhakar + - Thierry Reding + +allOf: + - $ref: panel-common.yaml# + - $ref: /schemas/display/lvds.yaml/# + +select: + properties: +compatible: + contains: +const: panel-lvds + + not: +properties: + compatible: +contains: + enum: +- advantech,idk-1110wr +- advantech,idk-2121wr +- innolux,ee101ia-01d +- mitsubishi,aa104xd12 +- mitsubishi,aa121td01 +- sgd,gktw70sdae4se + + required: +- compatible + +properties: + compatible: +items: + - enum: + - auo,b101ew05 + - tbs,a711-panel + + - const: panel-lvds + +unevaluatedProperties: false + +required: + - compatible + - data-mapping + - width-mm + - height-mm + - panel-timing + - port + +... diff --git a/MAINTAINERS b/MAINTAINERS index 368072da0a05..02001455949e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6080,6 +6080,7 @@ T:git git://anongit.freedesktop.org/drm/drm-misc S: Maintained F: drivers/gpu/drm/panel/panel-lvds.c F: Documentation/devicetree/bindings/display/lvds.yaml +F: Documentation/devicetree/bindings/display/panel/panel-lvds.yaml DRM DRIVER FOR MANTIX MLAF057WE51 PANELS M: Guido Günther -- 2.34.1
Re: [PATCH] dma-buf: Move sysfs work out of DMA-BUF export/release path
On Tue, Jan 11, 2022 at 11:58:07AM +0100, Christian König wrote: > > > This is also not a problem due to the high number of DMA-BUF > > > exports during launch time, as even a single export can be delayed for > > > an unpredictable amount of time. We cannot eliminate DMA-BUF exports > > > completely during app-launches and we are unfortunately seeing reports > > > of the exporting process occasionally sleeping long enough to cause > > > user-visible jankiness :( > > > > > > We also looked at whether any optimizations are possible from the > > > kernfs implementation side[1] but the semaphore is used quite extensively > > > and it looks like the best way forward would be to remove sysfs > > > creation/teardown from the DMA-BUF export/release path altogether. We > > > have some ideas on how we can reduce the code-complexity in the > > > current patch. If we manage to > > > simplify it considerably, would the approach of offloading sysfs > > > creation and teardown into a separate thread be acceptable Christian? > > At bare minimum I suggest to use a work_struct instead of re-inventing that > with kthread. > > And then only put the exporting of buffers into the background and not the > teardown. > > > > Thank you for the guidance! > > One worry I have here with doing this async that now userspace might > > have a dma-buf, but the sysfs entry does not yet exist, or the dma-buf > > is gone, but the sysfs entry still exists. That's a bit awkward wrt > > semantics. > > > > Also I'm pretty sure that if we can hit this, then other subsystems > > using kernfs have similar problems, so trying to fix this in kernfs > > with slightly more fine-grained locking sounds like a much more solid > > approach. The linked patch talks about how the big delays happen due > > to direct reclaim, and that might be limited to specific code paths > > that we need to look at? As-is this feels a bit much like papering > > over kernfs issues in hackish ways in sysfs users, instead of tackling > > the problem at its root. > > Which is exactly my feeling as well, yes. More and more people are using sysfs/kernfs now for things that it was never designed for (i.e. high-speed statistic gathering). That's not the fault of kernfs, it's the fault of people thinking it can be used for stuff like that :) But delays like this is odd, tearing down sysfs attributes should normally _never_ be a fast-path that matters to system throughput. So offloading it to a workqueue makes sense as the attributes here are for objects that are on the fast-path. thanks, greg k-h
[PATCH 1/3] drm/bridge: anx7625: Convert to use devm_kzalloc
Use devm_kzalloc instead of kzalloc and drop kfree(). Let the memory
handled by driver detach.
Signed-off-by: Hsin-Yi Wang
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 0b858c78abe8b6..dbe708eb3bcf11 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2515,7 +2515,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
return -ENODEV;
}
- platform = kzalloc(sizeof(*platform), GFP_KERNEL);
+ platform = devm_kzalloc(dev, sizeof(*platform), GFP_KERNEL);
if (!platform) {
DRM_DEV_ERROR(dev, "fail to allocate driver data\n");
return -ENOMEM;
@@ -2527,7 +2527,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (ret) {
if (ret != -EPROBE_DEFER)
DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
- goto free_platform;
+ return ret;
}
platform->client = client;
@@ -2552,7 +2552,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (!platform->hdcp_workqueue) {
dev_err(dev, "fail to create work queue\n");
ret = -ENOMEM;
- goto free_platform;
+ return ret;
}
platform->pdata.intp_irq = client->irq;
@@ -2637,9 +2637,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (platform->hdcp_workqueue)
destroy_workqueue(platform->hdcp_workqueue);
-free_platform:
- kfree(platform);
-
return ret;
}
--
2.34.1.575.g55b058a8bb-goog
[PATCH 2/3] drm/bridge: anx7625: Support reading edid through aux channel
Support reading edid through aux channel if panel is connected to aux
bus. Extend anx7625_aux_dpcd_trans() to implement aux transfer function:
1. panel is populated in devm_of_dp_aux_populate_ep_devices(), so move
anx7625_parse_dt() after.
2. Use pm runtime autosuspend since aux transfer function is called
multiple times when reading edid.
3. No-op if aux transfer length is 0.
Signed-off-by: Hsin-Yi Wang
---
This patch is based on drm-misc-next and depends on
https://patchwork.freedesktop.org/patch/469081/
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 123 ++
drivers/gpu/drm/bridge/analogix/anx7625.h | 4 +
2 files changed, 109 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index dbe708eb3bcf11..d60cfc058fbe7d 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -231,19 +232,22 @@ static int wait_aux_op_finish(struct anx7625_data *ctx)
return 0;
}
-static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx, u8 op,
- u32 address, u8 len, u8 *buf)
+static int anx7625_aux_trans(struct anx7625_data *ctx, u8 op, u32 address,
+u8 len, u8 *buf)
{
struct device *dev = &ctx->client->dev;
int ret;
u8 addrh, addrm, addrl;
u8 cmd;
- if (len > MAX_DPCD_BUFFER_SIZE) {
+ if (len > DP_AUX_MAX_PAYLOAD_BYTES) {
dev_err(dev, "exceed aux buffer len.\n");
return -EINVAL;
}
+ if (!len)
+ return len;
+
addrl = address & 0xFF;
addrm = (address >> 8) & 0xFF;
addrh = (address >> 16) & 0xFF;
@@ -262,7 +266,7 @@ static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx,
u8 op,
ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
AP_AUX_ADDR_19_16, addrh);
- if (op == DPCD_WRITE)
+ if (op == DPCD_WRITE || op == AP_AUX_WRITE)
ret |= anx7625_reg_block_write(ctx, ctx->i2c.rx_p0_client,
AP_AUX_BUFF_START, len, buf);
/* Enable aux access */
@@ -275,14 +279,14 @@ static int anx7625_aux_dpcd_trans(struct anx7625_data
*ctx, u8 op,
}
ret = wait_aux_op_finish(ctx);
- if (ret) {
+ if (ret < 0) {
dev_err(dev, "aux IO error: wait aux op finish.\n");
return ret;
}
/* Write done */
- if (op == DPCD_WRITE)
- return 0;
+ if (op == DPCD_WRITE || op == AP_AUX_WRITE)
+ return len;
/* Read done, read out dpcd data */
ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
@@ -292,7 +296,7 @@ static int anx7625_aux_dpcd_trans(struct anx7625_data *ctx,
u8 op,
return -EIO;
}
- return 0;
+ return len;
}
static int anx7625_video_mute_control(struct anx7625_data *ctx,
@@ -867,7 +871,7 @@ static int anx7625_hdcp_enable(struct anx7625_data *ctx)
}
/* Read downstream capability */
- anx7625_aux_dpcd_trans(ctx, DPCD_READ, 0x68028, 1, &bcap);
+ anx7625_aux_trans(ctx, DPCD_READ, 0x68028, 1, &bcap);
if (!(bcap & 0x01)) {
pr_warn("downstream not support HDCP 1.4, cap(%x).\n", bcap);
return 0;
@@ -956,7 +960,7 @@ static void anx7625_dp_stop(struct anx7625_data *ctx)
dev_dbg(dev, "notify downstream enter into standby\n");
/* Downstream monitor enter into standby mode */
data = 2;
- ret |= anx7625_aux_dpcd_trans(ctx, DPCD_WRITE, 0x000600, 1, &data);
+ ret |= anx7625_aux_trans(ctx, DPCD_WRITE, 0x000600, 1, &data);
if (ret < 0)
DRM_DEV_ERROR(dev, "IO error : mute video fail\n");
@@ -1655,11 +1659,60 @@ static int anx7625_parse_dt(struct device *dev,
return 0;
}
+static bool anx7625_of_panel_on_aux_bus(struct device *dev)
+{
+ struct device_node *bus, *panel;
+
+ bus = of_get_child_by_name(dev->of_node, "aux-bus");
+ if (!bus)
+ return false;
+
+ panel = of_get_child_by_name(bus, "panel");
+ of_node_put(bus);
+ if (!panel)
+ return false;
+ of_node_put(panel);
+
+ return true;
+}
+
static inline struct anx7625_data *bridge_to_anx7625(struct drm_bridge *bridge)
{
return container_of(bridge, struct anx7625_data, bridge);
}
+static ssize_t anx7625_aux_transfer(struct drm_dp_aux *aux,
+struct drm_dp_aux_msg *msg)
+{
+ struct anx7625_data *ctx = container_of(aux, struct anx7625_data, aux);
+ struct device *dev = &ctx->client->dev;
+ u8 request = msg->request & ~DP_AUX_I2C_MOT, op;
+ int ret = 0;
+
+ pm_runtime_get_sync(
[PATCH 3/3] dt-bindings: drm/bridge: anx7625: Add aux-bus node
List panel under aux-bus node if it's connected to anx7625's aux bus.
Signed-off-by: Hsin-Yi Wang
---
.../display/bridge/analogix,anx7625.yaml| 17 +
1 file changed, 17 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index 1d3e88daca041a..0d38d6fe39830f 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -83,6 +83,9 @@ properties:
type: boolean
description: let the driver enable audio HDMI codec function or not.
+ aux-bus:
+$ref: /schemas/display/dp-aux-bus.yaml#
+
ports:
$ref: /schemas/graph.yaml#/properties/ports
@@ -167,5 +170,19 @@ examples:
};
};
};
+
+aux-bus {
+panel {
+compatible = "innolux,n125hce-gn1";
+power-supply = <&pp3300_disp_x>;
+backlight = <&backlight_lcd0>;
+
+port {
+panel_in: endpoint {
+remote-endpoint = <&anx7625_out>;
+};
+};
+};
+};
};
};
--
2.34.1.575.g55b058a8bb-goog
Re: 回复: [PATCH] drm/ttm: Put BO in its memory manager's lru list
IIRC we have completely dropped this patch in favor of a check at a
different place.
Regards,
Christian.
Am 11.01.22 um 09:47 schrieb Chen, Guchun:
[Public]
Hi Christian,
Looks this patch still missed in 5.16 kernel. Is it intentional?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/ttm/ttm_bo.c?h=v5.16
Regards,
Guchun
-Original Message-
From: amd-gfx On Behalf Of Pan, Xinhui
Sent: Tuesday, November 9, 2021 9:16 PM
To: Koenig, Christian ; amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Subject: 回复: 回复: [PATCH] drm/ttm: Put BO in its memory manager's lru list
[AMD Official Use Only]
[AMD Official Use Only]
Actually this patch does not totally fix the mismatch of lru list with mem_type as
mem_type is changed in ->move() and lru list is changed after that.
During this small period, another eviction could still happed and evict this
mismatched BO from sMam(say, its lru list is on vram domain) to sMem.
发件人: Pan, Xinhui
发送时间: 2021年11月9日 21:05
收件人: Koenig, Christian; amd-...@lists.freedesktop.org
抄送: dri-devel@lists.freedesktop.org
主题: 回复: 回复: [PATCH] drm/ttm: Put BO in its memory manager's lru list
Yes, a stable tag is needed. vulkan guys say 5.14 hit this issue too.
I think that amdgpu_bo_move() does support copy from sysMem to sysMem correctly.
maybe something below is needed.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c83ef42ca702..aa63ae7ddf1e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -485,7 +485,8 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo,
bool evict,
}
if (old_mem->mem_type == TTM_PL_SYSTEM &&
(new_mem->mem_type == TTM_PL_TT ||
-new_mem->mem_type == AMDGPU_PL_PREEMPT)) {
+new_mem->mem_type == AMDGPU_PL_PREEMPT ||
+new_mem->mem_type == TTM_PL_SYSTEM)) {
ttm_bo_move_null(bo, new_mem);
goto out;
}
otherwise, amdgpu_move_blit() is called to do the system memory copy which use
a wrong address.
206 /* Map only what can't be accessed directly */
207 if (!tmz && mem->start != AMDGPU_BO_INVALID_OFFSET) {
208 *addr = amdgpu_ttm_domain_start(adev, mem->mem_type) +
209 mm_cur->start;
210 return 0;
211 }
line 208, *addr is zero. So when amdgpu_copy_buffer submit job with such addr,
page fault happens.
发件人: Koenig, Christian
发送时间: 2021年11月9日 20:35
收件人: Pan, Xinhui; amd-...@lists.freedesktop.org
抄送: dri-devel@lists.freedesktop.org
主题: Re: 回复: [PATCH] drm/ttm: Put BO in its memory manager's lru list
Mhm, I'm not sure what the rational behind that is.
Not moving the BO would make things less efficient, but should never cause a
crash.
Maybe we should add a CC: stable tag and push it to -fixes instead?
Christian.
Am 09.11.21 um 13:28 schrieb Pan, Xinhui:
[AMD Official Use Only]
I hit vulkan cts test hang with navi23.
dmesg says gmc page fault with address 0x0, 0x1000, 0x2000
And some debug log also says amdgu copy one BO from system Domain to system
Domain which is really weird.
发件人: Koenig, Christian
发送时间: 2021年11月9日 20:20
收件人: Pan, Xinhui; amd-...@lists.freedesktop.org
抄送: dri-devel@lists.freedesktop.org
主题: Re: [PATCH] drm/ttm: Put BO in its memory manager's lru list
Am 09.11.21 um 12:19 schrieb xinhui pan:
After we move BO to a new memory region, we should put it to the new
memory manager's lru list regardless we unlock the resv or not.
Signed-off-by: xinhui pan
Interesting find, did you trigger that somehow or did you just
stumbled over it by reading the code?
Patch is Reviewed-by: Christian König , I
will pick that up for drm-misc-next.
Thanks,
Christian.
---
drivers/gpu/drm/ttm/ttm_bo.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
b/drivers/gpu/drm/ttm/ttm_bo.c index f1367107925b..e307004f0b28
100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -701,6 +701,8 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
ret = ttm_bo_evict(bo, ctx);
if (locked)
ttm_bo_unreserve(bo);
+ else
+ ttm_bo_move_to_lru_tail_unlocked(bo);
ttm_bo_put(bo);
return ret;
Re: Phyr Starter
Hi
Am 10.01.22 um 20:34 schrieb Matthew Wilcox:
TLDR: I want to introduce a new data type:
struct phyr {
phys_addr_t addr;
size_t len;
};
Did you look at struct dma_buf_map? [1]
For graphics framebuffers, we have the problem that these buffers can be
in I/O or system memory (and possibly move between them). Linux'
traditional interfaces (memcpy_toio(), etc) don't deal with the
differences well.
So we added struct dma_buf_map as an abstraction to the buffer address.
There are interfaces for accessing and copying the data. I also have a
patchset somewhere that adds caching information to the structure.
struct dma_buf_map is for graphics, but really just another memory API.
When we introduced struct dma_buf_map we thought of additional use
cases, but couldn't really find any at the time. Maybe what you're
describing is that use case and struct dma_buf_map could be extended for
this purpose.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v5.16/source/include/linux/dma-buf-map.h#L115
and use it to replace bio_vec as well as using it to replace the array
of struct pages used by get_user_pages() and friends.
---
There are two distinct problems I want to address: doing I/O to memory
which does not have a struct page and efficiently doing I/O to large
blobs of physically contiguous memory, regardless of whether it has a
struct page. There are some other improvements which I regard as minor.
There are many types of memory that one might want to do I/O to that do
not have a struct page, some examples:
- Memory on a graphics card (or other PCI card, but gfx seems to be
the primary provider of DRAM on the PCI bus today)
- DAX, or other pmem (there are some fake pages today, but this is
mostly a workaround for the IO problem today)
- Guest memory being accessed from the hypervisor (KVM needs to
create structpages to make this happen. Xen doesn't ...)
All of these kinds of memories can be addressed by the CPU and so also
by a bus master. That is, there is a physical address that the CPU
can use which will address this memory, and there is a way to convert
that to a DMA address which can be programmed into another device.
There's no intent here to support memory which can be accessed by a
complex scheme like writing an address to a control register and then
accessing the memory through a FIFO; this is for memory which can be
accessed by DMA and CPU loads and stores.
For get_user_pages() and friends, we currently fill an array of struct
pages, each one representing PAGE_SIZE bytes. For an application that
is using 1GB hugepages, writing 2^18 entries is a significant overhead.
It also makes drivers hard to write as they have to recoalesce the
struct pages, even though the VM can tell it whether those 2^18 pages
are contiguous.
On the minor side, struct phyr can represent any mappable chunk of memory.
A bio_vec is limited to 2^32 bytes, while on 64-bit machines a phyr
can represent larger than 4GB. A phyr is the same size as a bio_vec
on 64 bit (16 bytes), and the same size for 32-bit with PAE (12 bytes).
It is smaller for 32-bit machines without PAE (8 bytes instead of 12).
Finally, it may be possible to stop using scatterlist to describe the
input to the DMA-mapping operation. We may be able to get struct
scatterlist down to just dma_address and dma_length, with chaining
handled through an enclosing struct.
I would like to see phyr replace bio_vec everywhere it's currently used.
I don't have time to do that work now because I'm busy with folios.
If someone else wants to take that on, I shall cheer from the sidelines.
What I do intend to do is:
- Add an interface to gup.c to pin/unpin N phyrs
- Add a sg_map_phyrs()
This will take an array of phyrs and allocate an sg for them
- Whatever else I need to do to make one RDMA driver happy with
this scheme
At that point, I intend to stop and let others more familiar with this
area of the kernel continue the conversion of drivers.
P.S. If you've had the Prodigy song running through your head the whole
time you've been reading this email ... I'm sorry / You're welcome.
If people insist, we can rename this to phys_range or something boring,
but I quite like the spelling of phyr with the pronunciation of "fire".
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
[Bug 215445] AMDGPU -- UBSAN: invalid-load in amdgpu_dm.c:5882:84 - load of value 32 is not a valid value for type '_Bool'
https://bugzilla.kernel.org/show_bug.cgi?id=215445 talktome7...@gmail.com changed: What|Removed |Added CC||talktome7...@gmail.com --- Comment #6 from talktome7...@gmail.com --- UBSAN is enabled for the 5.15 kernels since 5.15.8. It is not enabled for the 5.16 kernels. Check the build log files to confirm that ubsan.o is linked into the kernel for 5.15, but not 5.16. Also check out the discussion in https://gitlab.freedesktop.org/drm/amd/-/issues/1779. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH] dma-buf: Move sysfs work out of DMA-BUF export/release path
Am 11.01.22 um 12:16 schrieb Greg Kroah-Hartman: On Tue, Jan 11, 2022 at 11:58:07AM +0100, Christian König wrote: This is also not a problem due to the high number of DMA-BUF exports during launch time, as even a single export can be delayed for an unpredictable amount of time. We cannot eliminate DMA-BUF exports completely during app-launches and we are unfortunately seeing reports of the exporting process occasionally sleeping long enough to cause user-visible jankiness :( We also looked at whether any optimizations are possible from the kernfs implementation side[1] but the semaphore is used quite extensively and it looks like the best way forward would be to remove sysfs creation/teardown from the DMA-BUF export/release path altogether. We have some ideas on how we can reduce the code-complexity in the current patch. If we manage to simplify it considerably, would the approach of offloading sysfs creation and teardown into a separate thread be acceptable Christian? At bare minimum I suggest to use a work_struct instead of re-inventing that with kthread. And then only put the exporting of buffers into the background and not the teardown. Thank you for the guidance! One worry I have here with doing this async that now userspace might have a dma-buf, but the sysfs entry does not yet exist, or the dma-buf is gone, but the sysfs entry still exists. That's a bit awkward wrt semantics. Also I'm pretty sure that if we can hit this, then other subsystems using kernfs have similar problems, so trying to fix this in kernfs with slightly more fine-grained locking sounds like a much more solid approach. The linked patch talks about how the big delays happen due to direct reclaim, and that might be limited to specific code paths that we need to look at? As-is this feels a bit much like papering over kernfs issues in hackish ways in sysfs users, instead of tackling the problem at its root. Which is exactly my feeling as well, yes. More and more people are using sysfs/kernfs now for things that it was never designed for (i.e. high-speed statistic gathering). That's not the fault of kernfs, it's the fault of people thinking it can be used for stuff like that :) I'm starting to get the feeling that we should maybe have questioned adding sysfs files for each exported DMA-buf a bit more. Anyway, to late for that. We have to live with the consequences. But delays like this is odd, tearing down sysfs attributes should normally _never_ be a fast-path that matters to system throughput. So offloading it to a workqueue makes sense as the attributes here are for objects that are on the fast-path. That's what is puzzling me as well. As far as I understood Hridya tearing down things is not the problem, because during teardown we usually have a dying task where it's usually not much of a problem if the corpse is around for another few milliseconds until everything is cleaned up. The issue happens during creation of the sysfs attribute and that's extremely odd because if this waits for reclaim then drivers will certainly wait for reclaim as well. See we need a few bytes for the sysfs attribute, but drivers usually need a few megabytes for the DMA-buf backing store before they can even export the DMA-buf. So something doesn't add up in the rational for this problem. Regards, Christian. thanks, greg k-h
Re: [PATCH v4 0/6] drm: exynos: dsi: Convert drm bridge
Hi Jagan,
On 11.01.2022 10:32, Jagan Teki wrote:
Hi Andrzej,
On Tue, Dec 28, 2021 at 4:18 PM Andrzej Hajda wrote:
Hi Marek,
On 23.12.2021 10:15, Marek Szyprowski wrote:
Hi Jagan,
On 18.12.2021 00:16, Marek Szyprowski wrote:
On 15.12.2021 15:56, Jagan Teki wrote:
On Wed, Dec 15, 2021 at 7:49 PM Marek Szyprowski
wrote:
On 15.12.2021 13:57, Jagan Teki wrote:
On Wed, Dec 15, 2021 at 5:31 PM Marek Szyprowski
wrote:
On 15.12.2021 11:15, Jagan Teki wrote:
Updated series about drm bridge conversion of exynos dsi.
Previous version can be accessible, here [1].
Patch 1: connector reset
Patch 2: panel_bridge API
Patch 3: Bridge conversion
Patch 4: Atomic functions
Patch 5: atomic_set
Patch 6: DSI init in enable
There is a little progress! :)
Devices with a simple display pipeline (only a DSI panel, like
Trats/Trats2) works till the last patch. Then, after applying
("[PATCH
v4 6/6] drm: exynos: dsi: Move DSI init in bridge enable"), I get no
display at all.
A TM2e board with in-bridge (Exynos MIC) stops displaying anything
after
applying patch "[PATCH v4 2/6] drm: exynos: dsi: Use drm
panel_bridge API".
In case of the Arndale board with tc358764 bridge, no much
progress. The
display is broken just after applying the "[PATCH v2] drm: bridge:
tc358764: Use drm panel_bridge API" patch on top of linux-next.
In all cases the I had "drm: of: Lookup if child node has panel or
bridge" patch applied.
Just skip the 6/6 for now.
Apply
-
https://protect2.fireeye.com/v1/url?k=a24f3f76-fdd40659-a24eb439-0cc47a31cdf8-97ea12b4c5258d11&q=1&e=37a169bf-7ca5-4362-aad7-486018c7a708&u=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fpatch%2F1825%2F
-
https://protect2.fireeye.com/v1/url?k=a226360f-fdbd0f20-a227bd40-0cc47a31cdf8-ebd66aebee1058d7&q=1&e=37a169bf-7ca5-4362-aad7-486018c7a708&u=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fpatch%2F1823%2F
Then apply 1/6 to 5/6. and update the status?
Okay, my fault, I didn't check that case on Arndale.
I've checked and indeed, Trats/Trats2 and Arndale works after the above
2 patches AND patches 1-5.
The only problem is now on TM2e, which uses Exynos MIC as in-bridge for
Exynos DSI:
[4.068866] [drm] Exynos DRM: using 1380.decon device for DMA
mapping operations
[4.069183] exynos-drm exynos-drm: bound 1380.decon (ops
decon_component_ops)
[4.128983] exynos-drm exynos-drm: bound 1388.decon (ops
decon_component_ops)
[4.129261] exynos-drm exynos-drm: bound 1393.mic (ops
exynos_mic_component_ops)
[4.133508] exynos-dsi 1390.dsi: [drm:exynos_dsi_host_attach]
*ERROR* failed to find the bridge: -19
[4.136392] exynos-drm exynos-drm: bound 1390.dsi (ops
exynos_dsi_component_ops)
[4.145499] rc_core: Couldn't load IR keymap rc-cec
[4.145666] Registered IR keymap rc-empty
[4.148402] rc rc0: sii8620 as /devices/virtual/rc/rc0
[4.156051] input: sii8620 as /devices/virtual/rc/rc0/input1
[4.160647] exynos-drm exynos-drm: bound 1397.hdmi (ops
hdmi_component_ops)
[4.169923] exynos-drm exynos-drm: [drm] Cannot find any crtc or
sizes
[4.173958] exynos-drm exynos-drm: [drm] Cannot find any crtc or
sizes
[4.182304] [drm] Initialized exynos 1.1.0 20180330 for
exynos-drm on
minor 0
The display pipeline for TM2e is:
Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel
If Trats/Trats2 is working then it has to work. I don't see any
difference in output pipeline. Can you please share the full log, I
cannot see host_attach print saying "Attached.."
Well, there is a failure message about the panel:
exynos-dsi 1390.dsi: [drm:exynos_dsi_host_attach] *ERROR* failed
to find the bridge: -19
however it looks that something might be broken in dts. The in-bridge
(Exynos MIC) is on port 0 and the panel is @0, what imho might cause
the issue.
I've tried to change in in-bridge ('mic_to_dsi') port to 1 in
exynos5433.dtsi. Then the panel has been attached:
exynos-dsi 1390.dsi: [drm:exynos_dsi_host_attach] Attached s6e3hf2
device
but the display is still not working, probably due to lack of proper
Exynos MIC handling. I will investigate it later and let You know.
I've played a bit with the Exynos DRM code and finally I made it working
on TM2(e). There are basically 3 different issues that need to be fixed
to get it working with the $subject patchset:
1. Port numbers in exynos5433 dsi/dts are broken. For all pre-Exynos5433
boards the panel was defined as a DSI node child (at 'reg 0'),
True, emphasis that it is reg 0 of DSI bus.
what
means it used port 0.
And this does not seems true to me. Established practice is (unless I
have not noticed change in bindings :) ) that in case of data bus shared
with control bus the port node is optional. In such case host knows
already who is connected to its data bus, it does not need port node.
And if there is no port node there is no port number as well.
As I quickly looked at the exynos bindings they seems generally OK to
me, if there is something
[PATCH 1/8] drm/ast: Fail if connector initialization fails
Update the connector code to fail if the connector could not be initialized. The current code just ignored the error and failed later when the connector was supposed to be used. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_mode.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 956c8982192b..20626c78a693 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1319,18 +1319,21 @@ static int ast_connector_init(struct drm_device *dev) struct ast_connector *ast_connector = &ast->connector; struct drm_connector *connector = &ast_connector->base; struct drm_encoder *encoder = &ast->encoder; + int ret; ast_connector->i2c = ast_i2c_create(dev); if (!ast_connector->i2c) drm_err(dev, "failed to add ddc bus for connector\n"); if (ast_connector->i2c) - drm_connector_init_with_ddc(dev, connector, &ast_connector_funcs, - DRM_MODE_CONNECTOR_VGA, - &ast_connector->i2c->adapter); + ret = drm_connector_init_with_ddc(dev, connector, &ast_connector_funcs, + DRM_MODE_CONNECTOR_VGA, + &ast_connector->i2c->adapter); else - drm_connector_init(dev, connector, &ast_connector_funcs, - DRM_MODE_CONNECTOR_VGA); + ret = drm_connector_init(dev, connector, &ast_connector_funcs, +DRM_MODE_CONNECTOR_VGA); + if (ret) + return ret; drm_connector_helper_add(connector, &ast_connector_helper_funcs); -- 2.34.1
[PATCH 0/8] drm/ast: Untangle connector helpers
The ast driver supports different transmitter chips to support DVI and HDMI connectors. It's al been packed into the same helpers functons and exported as VGA connector. Introduce a separate set of connector helpers for each transmitter chip, and thus connector type. Also provide the correct encoder for each connector. This change mostly affects the connector's get_modes helper, where VGA-, DVI- and HDMI-related code was lumped into the same function. It's now all separate. While at it, also rework and cleanup the initialization of the related data structures. Tested on AST 2100 and AST 2300 hardware. I don't have hardware with special transmitter chips (SIL164, DP501), so I could only test the VGA code. Thomas Zimmermann (8): drm/ast: Fail if connector initialization fails drm/ast: Move connector mode_valid function to CRTC drm/ast: Remove AST_TX_ITE66121 constant drm/ast: Remove unused value dp501_maxclk drm/ast: Rename struct ast_connector to struct ast_vga_connector drm/ast: Initialize encoder and connector for VGA in helper function drm/ast: Move DP501-based connector code into separate helpers drm/ast: Move SIL164-based connector code into separate helpers drivers/gpu/drm/ast/ast_dp501.c | 58 - drivers/gpu/drm/ast/ast_drv.h | 37 ++- drivers/gpu/drm/ast/ast_mode.c | 413 +++- 3 files changed, 331 insertions(+), 177 deletions(-) base-commit: fbce7b8d8df5af8d404b6aeaf63779f91bdbeb5d prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 -- 2.34.1
[PATCH 6/8] drm/ast: Initialize encoder and connector for VGA in helper function
Move encoder and connector initialization into a single helper and
put all related mode-setting structures into a single place. Done in
preparation of moving transmitter code into separate helpers. No
functional changes.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_drv.h | 8 +++--
drivers/gpu/drm/ast/ast_mode.c | 62 --
2 files changed, 42 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index e1cb31acdaac..cda50fb887ed 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -160,8 +160,12 @@ struct ast_private {
struct drm_plane primary_plane;
struct ast_cursor_plane cursor_plane;
struct drm_crtc crtc;
- struct drm_encoder encoder;
- struct ast_vga_connector connector;
+ union {
+ struct {
+ struct drm_encoder encoder;
+ struct ast_vga_connector vga_connector;
+ } vga;
+ } output;
bool support_wide_screen;
enum {
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index f7f4034cc91e..17e4e038a3ed 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1249,25 +1249,6 @@ static int ast_crtc_init(struct drm_device *dev)
return 0;
}
-/*
- * Encoder
- */
-
-static int ast_encoder_init(struct drm_device *dev)
-{
- struct ast_private *ast = to_ast_private(dev);
- struct drm_encoder *encoder = &ast->encoder;
- int ret;
-
- ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_DAC);
- if (ret)
- return ret;
-
- encoder->possible_crtcs = 1;
-
- return 0;
-}
-
/*
* VGA Connector
*/
@@ -1315,12 +1296,10 @@ static const struct drm_connector_funcs
ast_vga_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
-static int ast_vga_connector_init(struct drm_device *dev)
+static int ast_vga_connector_init(struct drm_device *dev,
+ struct ast_vga_connector *ast_vga_connector)
{
- struct ast_private *ast = to_ast_private(dev);
- struct ast_vga_connector *ast_vga_connector = &ast->connector;
struct drm_connector *connector = &ast_vga_connector->base;
- struct drm_encoder *encoder = &ast->encoder;
int ret;
ast_vga_connector->i2c = ast_i2c_create(dev);
@@ -1344,7 +1323,30 @@ static int ast_vga_connector_init(struct drm_device *dev)
connector->polled = DRM_CONNECTOR_POLL_CONNECT;
- drm_connector_attach_encoder(connector, encoder);
+ return 0;
+}
+
+static int ast_vga_output_init(struct ast_private *ast)
+{
+ struct drm_device *dev = &ast->base;
+ struct drm_crtc *crtc = &ast->crtc;
+ struct drm_encoder *encoder = &ast->output.vga.encoder;
+ struct ast_vga_connector *ast_vga_connector =
&ast->output.vga.vga_connector;
+ struct drm_connector *connector = &ast_vga_connector->base;
+ int ret;
+
+ ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_DAC);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ ret = ast_vga_connector_init(dev, ast_vga_connector);
+ if (ret)
+ return ret;
+
+ ret = drm_connector_attach_encoder(connector, encoder);
+ if (ret)
+ return ret;
return 0;
}
@@ -1405,8 +1407,16 @@ int ast_mode_config_init(struct ast_private *ast)
return ret;
ast_crtc_init(dev);
- ast_encoder_init(dev);
- ast_vga_connector_init(dev);
+
+ switch (ast->tx_chip_type) {
+ case AST_TX_NONE:
+ case AST_TX_SIL164:
+ case AST_TX_DP501:
+ ret = ast_vga_output_init(ast);
+ break;
+ }
+ if (ret)
+ return ret;
drm_mode_config_reset(dev);
--
2.34.1
[PATCH 5/8] drm/ast: Rename struct ast_connector to struct ast_vga_connector
Prepare for introducing other connectors besides VGA. No functional
changes.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_drv.h | 10
drivers/gpu/drm/ast/ast_mode.c | 45 +-
2 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 479bb120dd05..e1cb31acdaac 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -129,15 +129,15 @@ struct ast_i2c_chan {
struct i2c_algo_bit_data bit;
};
-struct ast_connector {
+struct ast_vga_connector {
struct drm_connector base;
struct ast_i2c_chan *i2c;
};
-static inline struct ast_connector *
-to_ast_connector(struct drm_connector *connector)
+static inline struct ast_vga_connector *
+to_ast_vga_connector(struct drm_connector *connector)
{
- return container_of(connector, struct ast_connector, base);
+ return container_of(connector, struct ast_vga_connector, base);
}
/*
@@ -161,7 +161,7 @@ struct ast_private {
struct ast_cursor_plane cursor_plane;
struct drm_crtc crtc;
struct drm_encoder encoder;
- struct ast_connector connector;
+ struct ast_vga_connector connector;
bool support_wide_screen;
enum {
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 0a8aa6e3aa38..f7f4034cc91e 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1269,12 +1269,12 @@ static int ast_encoder_init(struct drm_device *dev)
}
/*
- * Connector
+ * VGA Connector
*/
-static int ast_get_modes(struct drm_connector *connector)
+static int ast_vga_connector_helper_get_modes(struct drm_connector *connector)
{
- struct ast_connector *ast_connector = to_ast_connector(connector);
+ struct ast_vga_connector *ast_vga_connector =
to_ast_vga_connector(connector);
struct ast_private *ast = to_ast_private(connector->dev);
struct edid *edid = NULL;
bool flags = false;
@@ -1291,23 +1291,23 @@ static int ast_get_modes(struct drm_connector
*connector)
edid = NULL;
}
}
- if (!flags && ast_connector->i2c)
- edid = drm_get_edid(connector, &ast_connector->i2c->adapter);
+ if (!flags && ast_vga_connector->i2c)
+ edid = drm_get_edid(connector,
&ast_vga_connector->i2c->adapter);
if (edid) {
- drm_connector_update_edid_property(&ast_connector->base, edid);
+ drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
return ret;
}
- drm_connector_update_edid_property(&ast_connector->base, NULL);
+ drm_connector_update_edid_property(connector, NULL);
return 0;
}
-static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
- .get_modes = ast_get_modes,
+static const struct drm_connector_helper_funcs ast_vga_connector_helper_funcs
= {
+ .get_modes = ast_vga_connector_helper_get_modes,
};
-static const struct drm_connector_funcs ast_connector_funcs = {
+static const struct drm_connector_funcs ast_vga_connector_funcs = {
.reset = drm_atomic_helper_connector_reset,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = drm_connector_cleanup,
@@ -1315,29 +1315,29 @@ static const struct drm_connector_funcs
ast_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
-static int ast_connector_init(struct drm_device *dev)
+static int ast_vga_connector_init(struct drm_device *dev)
{
struct ast_private *ast = to_ast_private(dev);
- struct ast_connector *ast_connector = &ast->connector;
- struct drm_connector *connector = &ast_connector->base;
+ struct ast_vga_connector *ast_vga_connector = &ast->connector;
+ struct drm_connector *connector = &ast_vga_connector->base;
struct drm_encoder *encoder = &ast->encoder;
int ret;
- ast_connector->i2c = ast_i2c_create(dev);
- if (!ast_connector->i2c)
+ ast_vga_connector->i2c = ast_i2c_create(dev);
+ if (!ast_vga_connector->i2c)
drm_err(dev, "failed to add ddc bus for connector\n");
- if (ast_connector->i2c)
- ret = drm_connector_init_with_ddc(dev, connector,
&ast_connector_funcs,
+ if (ast_vga_connector->i2c)
+ ret = drm_connector_init_with_ddc(dev, connector,
&ast_vga_connector_funcs,
DRM_MODE_CONNECTOR_VGA,
- &ast_connector->i2c->adapter);
+
&ast_vga_connector->i2c->adapter);
else
- ret = drm_connector_init(dev, connector, &ast_connector_funcs,
+ ret = drm
[PATCH 4/8] drm/ast: Remove unused value dp501_maxclk
Remove reading the link-rate. The value is maintained by the connector
code but never used.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_dp501.c | 58 -
drivers/gpu/drm/ast/ast_drv.h | 1 -
drivers/gpu/drm/ast/ast_mode.c | 7 ++--
3 files changed, 3 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index cd93c44f2662..204c926a18ea 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -272,64 +272,6 @@ static bool ast_launch_m68k(struct drm_device *dev)
return true;
}
-u8 ast_get_dp501_max_clk(struct drm_device *dev)
-{
- struct ast_private *ast = to_ast_private(dev);
- u32 boot_address, offset, data;
- u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
- u32 *plinkcap;
-
- if (ast->config_mode == ast_use_p2a) {
- boot_address = get_fw_base(ast);
-
- /* validate FW version */
- offset = AST_DP501_GBL_VERSION;
- data = ast_mindwm(ast, boot_address + offset);
- if ((data & AST_DP501_FW_VERSION_MASK) !=
AST_DP501_FW_VERSION_1) /* version: 1x */
- return maxclk;
-
- /* Read Link Capability */
- offset = AST_DP501_LINKRATE;
- plinkcap = (u32 *)linkcap;
- *plinkcap = ast_mindwm(ast, boot_address + offset);
- if (linkcap[2] == 0) {
- linkrate = linkcap[0];
- linklanes = linkcap[1];
- data = (linkrate == 0x0a) ? (90 * linklanes) : (54 *
linklanes);
- if (data > 0xff)
- data = 0xff;
- maxclk = (u8)data;
- }
- } else {
- if (!ast->dp501_fw_buf)
- return AST_DP501_DEFAULT_DCLK; /* 1024x768 as default
*/
-
- /* dummy read */
- offset = 0x;
- data = readl(ast->dp501_fw_buf + offset);
-
- /* validate FW version */
- offset = AST_DP501_GBL_VERSION;
- data = readl(ast->dp501_fw_buf + offset);
- if ((data & AST_DP501_FW_VERSION_MASK) !=
AST_DP501_FW_VERSION_1) /* version: 1x */
- return maxclk;
-
- /* Read Link Capability */
- offset = AST_DP501_LINKRATE;
- plinkcap = (u32 *)linkcap;
- *plinkcap = readl(ast->dp501_fw_buf + offset);
- if (linkcap[2] == 0) {
- linkrate = linkcap[0];
- linklanes = linkcap[1];
- data = (linkrate == 0x0a) ? (90 * linklanes) : (54 *
linklanes);
- if (data > 0xff)
- data = 0xff;
- maxclk = (u8)data;
- }
- }
- return maxclk;
-}
-
bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
{
struct ast_private *ast = to_ast_private(dev);
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 6e77be1d06d3..479bb120dd05 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -171,7 +171,6 @@ struct ast_private {
} config_mode;
enum ast_tx_chip tx_chip_type;
- u8 dp501_maxclk;
u8 *dp501_fw_addr;
const struct firmware *dp501_fw;/* dp501 fw */
};
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index c555960a488a..0a8aa6e3aa38 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1281,16 +1281,15 @@ static int ast_get_modes(struct drm_connector
*connector)
int ret;
if (ast->tx_chip_type == AST_TX_DP501) {
- ast->dp501_maxclk = 0xff;
edid = kmalloc(128, GFP_KERNEL);
if (!edid)
return -ENOMEM;
flags = ast_dp501_read_edid(connector->dev, (u8 *)edid);
- if (flags)
- ast->dp501_maxclk =
ast_get_dp501_max_clk(connector->dev);
- else
+ if (!flags) {
kfree(edid);
+ edid = NULL;
+ }
}
if (!flags && ast_connector->i2c)
edid = drm_get_edid(connector, &ast_connector->i2c->adapter);
--
2.34.1
[PATCH 3/8] drm/ast: Remove AST_TX_ITE66121 constant
The ITE66121 is an HDMI transmitter chip. There's no code for
detecting or programming the chip within ast. Remove the enum
constant.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_drv.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 00bfa41ff7cb..6e77be1d06d3 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -69,7 +69,6 @@ enum ast_chip {
enum ast_tx_chip {
AST_TX_NONE,
AST_TX_SIL164,
- AST_TX_ITE66121,
AST_TX_DP501,
};
--
2.34.1
[PATCH 2/8] drm/ast: Move connector mode_valid function to CRTC
The tests in ast_mode_valid() verify the correct resolution for the
supplied mode. This is a limitation of the CRTC, so move the function
to the CRTC helpers. No functional changes.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_mode.c | 129 +
1 file changed, 66 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 20626c78a693..c555960a488a 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1002,6 +1002,71 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int
mode)
}
}
+static enum drm_mode_status
+ast_crtc_helper_mode_valid(struct drm_crtc *crtc, const struct
drm_display_mode *mode)
+{
+ struct ast_private *ast = to_ast_private(crtc->dev);
+ enum drm_mode_status status;
+ uint32_t jtemp;
+
+ if (ast->support_wide_screen) {
+ if ((mode->hdisplay == 1680) && (mode->vdisplay == 1050))
+ return MODE_OK;
+ if ((mode->hdisplay == 1280) && (mode->vdisplay == 800))
+ return MODE_OK;
+ if ((mode->hdisplay == 1440) && (mode->vdisplay == 900))
+ return MODE_OK;
+ if ((mode->hdisplay == 1360) && (mode->vdisplay == 768))
+ return MODE_OK;
+ if ((mode->hdisplay == 1600) && (mode->vdisplay == 900))
+ return MODE_OK;
+
+ if ((ast->chip == AST2100) || (ast->chip == AST2200) ||
+ (ast->chip == AST2300) || (ast->chip == AST2400) ||
+ (ast->chip == AST2500)) {
+ if ((mode->hdisplay == 1920) && (mode->vdisplay ==
1080))
+ return MODE_OK;
+
+ if ((mode->hdisplay == 1920) && (mode->vdisplay ==
1200)) {
+ jtemp = ast_get_index_reg_mask(ast,
AST_IO_CRTC_PORT, 0xd1, 0xff);
+ if (jtemp & 0x01)
+ return MODE_NOMODE;
+ else
+ return MODE_OK;
+ }
+ }
+ }
+
+ status = MODE_NOMODE;
+
+ switch (mode->hdisplay) {
+ case 640:
+ if (mode->vdisplay == 480)
+ status = MODE_OK;
+ break;
+ case 800:
+ if (mode->vdisplay == 600)
+ status = MODE_OK;
+ break;
+ case 1024:
+ if (mode->vdisplay == 768)
+ status = MODE_OK;
+ break;
+ case 1280:
+ if (mode->vdisplay == 1024)
+ status = MODE_OK;
+ break;
+ case 1600:
+ if (mode->vdisplay == 1200)
+ status = MODE_OK;
+ break;
+ default:
+ break;
+ }
+
+ return status;
+}
+
static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
@@ -1104,6 +1169,7 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
}
static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
+ .mode_valid = ast_crtc_helper_mode_valid,
.atomic_check = ast_crtc_helper_atomic_check,
.atomic_flush = ast_crtc_helper_atomic_flush,
.atomic_enable = ast_crtc_helper_atomic_enable,
@@ -1238,71 +1304,8 @@ static int ast_get_modes(struct drm_connector *connector)
return 0;
}
-static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
- struct drm_display_mode *mode)
-{
- struct ast_private *ast = to_ast_private(connector->dev);
- int flags = MODE_NOMODE;
- uint32_t jtemp;
-
- if (ast->support_wide_screen) {
- if ((mode->hdisplay == 1680) && (mode->vdisplay == 1050))
- return MODE_OK;
- if ((mode->hdisplay == 1280) && (mode->vdisplay == 800))
- return MODE_OK;
- if ((mode->hdisplay == 1440) && (mode->vdisplay == 900))
- return MODE_OK;
- if ((mode->hdisplay == 1360) && (mode->vdisplay == 768))
- return MODE_OK;
- if ((mode->hdisplay == 1600) && (mode->vdisplay == 900))
- return MODE_OK;
-
- if ((ast->chip == AST2100) || (ast->chip == AST2200) ||
- (ast->chip == AST2300) || (ast->chip == AST2400) ||
- (ast->chip == AST2500)) {
- if ((mode->hdisplay == 1920) && (mode->vdisplay ==
1080))
- return MODE_OK;
-
- if ((mode->hdisplay == 1920) && (mode->vdisplay ==
1200)) {
- jtemp = ast_get_index_reg_mask(ast,
AST_IO_CRT
[PATCH 7/8] drm/ast: Move DP501-based connector code into separate helpers
Add helpers for DP501-based connectors. DP501 provides output via
DisplayPort. This used to be handled by the VGA connector code.
If a DP501 chip has been detected, ast will now create a DisplayPort
connector instead of a VGA connector.
Remove the DP501 code from ast_vga_connector_helper_get_modes(). Also
remove the call to drm_connector_update_edid_property(), which is
performed by drm_get_edid().
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_drv.h | 4 ++
drivers/gpu/drm/ast/ast_mode.c | 128 +++--
2 files changed, 109 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index cda50fb887ed..420d19d8459e 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -165,6 +165,10 @@ struct ast_private {
struct drm_encoder encoder;
struct ast_vga_connector vga_connector;
} vga;
+ struct {
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+ } dp501;
} output;
bool support_wide_screen;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 17e4e038a3ed..a0f4f042141e 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1256,30 +1257,22 @@ static int ast_crtc_init(struct drm_device *dev)
static int ast_vga_connector_helper_get_modes(struct drm_connector *connector)
{
struct ast_vga_connector *ast_vga_connector =
to_ast_vga_connector(connector);
- struct ast_private *ast = to_ast_private(connector->dev);
- struct edid *edid = NULL;
- bool flags = false;
- int ret;
+ struct edid *edid;
+ int count;
- if (ast->tx_chip_type == AST_TX_DP501) {
- edid = kmalloc(128, GFP_KERNEL);
- if (!edid)
- return -ENOMEM;
+ if (!ast_vga_connector->i2c)
+ goto err_drm_connector_update_edid_property;
- flags = ast_dp501_read_edid(connector->dev, (u8 *)edid);
- if (!flags) {
- kfree(edid);
- edid = NULL;
- }
- }
- if (!flags && ast_vga_connector->i2c)
- edid = drm_get_edid(connector,
&ast_vga_connector->i2c->adapter);
- if (edid) {
- drm_connector_update_edid_property(connector, edid);
- ret = drm_add_edid_modes(connector, edid);
- kfree(edid);
- return ret;
- }
+ edid = drm_get_edid(connector, &ast_vga_connector->i2c->adapter);
+ if (!edid)
+ goto err_drm_connector_update_edid_property;
+
+ count = drm_add_edid_modes(connector, edid);
+ kfree(edid);
+
+ return count;
+
+err_drm_connector_update_edid_property:
drm_connector_update_edid_property(connector, NULL);
return 0;
}
@@ -1351,6 +1344,92 @@ static int ast_vga_output_init(struct ast_private *ast)
return 0;
}
+/*
+ * DP501 Connector
+ */
+
+static int ast_dp501_connector_helper_get_modes(struct drm_connector
*connector)
+{
+ void *edid;
+ bool succ;
+ int count;
+
+ edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
+ if (!edid)
+ goto err_drm_connector_update_edid_property;
+
+ succ = ast_dp501_read_edid(connector->dev, edid);
+ if (!succ)
+ goto err_kfree;
+
+ drm_connector_update_edid_property(connector, edid);
+ count = drm_add_edid_modes(connector, edid);
+ kfree(edid);
+
+ return count;
+
+err_kfree:
+ kfree(edid);
+err_drm_connector_update_edid_property:
+ drm_connector_update_edid_property(connector, NULL);
+ return 0;
+}
+
+static const struct drm_connector_helper_funcs
ast_dp501_connector_helper_funcs = {
+ .get_modes = ast_dp501_connector_helper_get_modes,
+};
+
+static const struct drm_connector_funcs ast_dp501_connector_funcs = {
+ .reset = drm_atomic_helper_connector_reset,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int ast_dp501_connector_init(struct drm_device *dev, struct
drm_connector *connector)
+{
+ int ret;
+
+ ret = drm_connector_init(dev, connector, &ast_dp501_connector_funcs,
+DRM_MODE_CONNECTOR_DisplayPort);
+ if (ret)
+ return ret;
+
+ drm_connector_helper_add(connector, &ast_dp501_connector_helper_funcs);
+
+ connector->interlace_allowed = 0;
+ connector->doublescan_allowed = 0;
+
+ connector->polled = DRM_CONNEC
[PATCH 8/8] drm/ast: Move SIL164-based connector code into separate helpers
Add helpers for initializing SIL164-based connectors. These used to be
handled by the VGA connector code. But SIL164 provides output via DVI-I,
so set the encoder and connector types accordingly.
If a SIL164 chip has been detected, ast will now create a DVI-I
connector instead of a VGA connector.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_drv.h | 15 ++
drivers/gpu/drm/ast/ast_mode.c | 99 +-
2 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 420d19d8459e..c3a582372649 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -140,6 +140,17 @@ to_ast_vga_connector(struct drm_connector *connector)
return container_of(connector, struct ast_vga_connector, base);
}
+struct ast_sil164_connector {
+ struct drm_connector base;
+ struct ast_i2c_chan *i2c;
+};
+
+static inline struct ast_sil164_connector *
+to_ast_sil164_connector(struct drm_connector *connector)
+{
+ return container_of(connector, struct ast_sil164_connector, base);
+}
+
/*
* Device
*/
@@ -165,6 +176,10 @@ struct ast_private {
struct drm_encoder encoder;
struct ast_vga_connector vga_connector;
} vga;
+ struct {
+ struct drm_encoder encoder;
+ struct ast_sil164_connector sil164_connector;
+ } sil164;
struct {
struct drm_encoder encoder;
struct drm_connector connector;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index a0f4f042141e..f9daeb8d801a 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1344,6 +1344,100 @@ static int ast_vga_output_init(struct ast_private *ast)
return 0;
}
+/*
+ * SIL164 Connector
+ */
+
+static int ast_sil164_connector_helper_get_modes(struct drm_connector
*connector)
+{
+ struct ast_sil164_connector *ast_sil164_connector =
to_ast_sil164_connector(connector);
+ struct edid *edid;
+ int count;
+
+ if (!ast_sil164_connector->i2c)
+ goto err_drm_connector_update_edid_property;
+
+ edid = drm_get_edid(connector, &ast_sil164_connector->i2c->adapter);
+ if (!edid)
+ goto err_drm_connector_update_edid_property;
+
+ count = drm_add_edid_modes(connector, edid);
+ kfree(edid);
+
+ return count;
+
+err_drm_connector_update_edid_property:
+ drm_connector_update_edid_property(connector, NULL);
+ return 0;
+}
+
+static const struct drm_connector_helper_funcs
ast_sil164_connector_helper_funcs = {
+ .get_modes = ast_sil164_connector_helper_get_modes,
+};
+
+static const struct drm_connector_funcs ast_sil164_connector_funcs = {
+ .reset = drm_atomic_helper_connector_reset,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int ast_sil164_connector_init(struct drm_device *dev,
+struct ast_sil164_connector
*ast_sil164_connector)
+{
+ struct drm_connector *connector = &ast_sil164_connector->base;
+ int ret;
+
+ ast_sil164_connector->i2c = ast_i2c_create(dev);
+ if (!ast_sil164_connector->i2c)
+ drm_err(dev, "failed to add ddc bus for connector\n");
+
+ if (ast_sil164_connector->i2c)
+ ret = drm_connector_init_with_ddc(dev, connector,
&ast_sil164_connector_funcs,
+ DRM_MODE_CONNECTOR_DVII,
+
&ast_sil164_connector->i2c->adapter);
+ else
+ ret = drm_connector_init(dev, connector,
&ast_sil164_connector_funcs,
+DRM_MODE_CONNECTOR_DVII);
+ if (ret)
+ return ret;
+
+ drm_connector_helper_add(connector, &ast_sil164_connector_helper_funcs);
+
+ connector->interlace_allowed = 0;
+ connector->doublescan_allowed = 0;
+
+ connector->polled = DRM_CONNECTOR_POLL_CONNECT;
+
+ return 0;
+}
+
+static int ast_sil164_output_init(struct ast_private *ast)
+{
+ struct drm_device *dev = &ast->base;
+ struct drm_crtc *crtc = &ast->crtc;
+ struct drm_encoder *encoder = &ast->output.sil164.encoder;
+ struct ast_sil164_connector *ast_sil164_connector =
&ast->output.sil164.sil164_connector;
+ struct drm_connector *connector = &ast_sil164_connector->base;
+ int ret;
+
+ ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_TMDS);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+
Re: [PATCH v2 1/2] drm/mipi-dbi: Remove dependency on GEM CMA helper library
> The MIPI DBI helpers access struct drm_gem_cma_object.vaddr in a
> few places. Replace all instances with the correct generic GEM
> functions. Use drm_gem_fb_vmap() for mapping a framebuffer's GEM
> objects and drm_gem_fb_vunmap() for unmapping them. This removes
> the dependency on CMA helpers within MIPI DBI.
>
> Signed-off-by: Thomas Zimmermann
> Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_mipi_dbi.c | 34 +-
> 1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c
b/drivers/gpu/drm/drm_mipi_dbi.c
> index 71b646c4131f..f80fd6c0ccf8 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -15,9 +15,10 @@
> #include
> #include
> #include
> -#include
> +#include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -200,13 +201,19 @@ int mipi_dbi_buf_copy(void *dst, struct
drm_framebuffer *fb,
> struct drm_rect *clip, bool swap)
> {
> struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
> - struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(gem);
> - void *src = cma_obj->vaddr;
> + struct dma_buf_map map[DRM_FORMAT_MAX_PLANES];
> + struct dma_buf_map data[DRM_FORMAT_MAX_PLANES];
> + void *src;
> int ret;
>
> ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> if (ret)
> return ret;
> + src = data[0].vaddr; /* TODO: Use mapping abstraction properly */
This assignment should be after the _vmap() call. The MIPI DBI drivers
are currently broken because of this.
Noralf.
> +
> + ret = drm_gem_fb_vmap(fb, map, data);
> + if (ret)
> + goto out_drm_gem_fb_end_cpu_access;
>
> switch (fb->format->format) {
> case DRM_FORMAT_RGB565:
> @@ -221,9 +228,11 @@ int mipi_dbi_buf_copy(void *dst, struct
drm_framebuffer *fb,
> default:
> drm_err_once(fb->dev, "Format is not supported: %p4cc\n",
>&fb->format->format);
> - return -EINVAL;
> + ret = -EINVAL;
> }
>
> + drm_gem_fb_vunmap(fb, map);
> +out_drm_gem_fb_end_cpu_access:
> drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>
> return ret;
>
[PATCH 0/2] Introduce multitile support
Hi, This is the second series that prepares i915 to host multitile platforms. It introduces the for_each_gt() macro that loops over the tiles to perform per gt actions. This patch is a combination of two patches developed originally by Abdiel, who introduced some refactoring during probe, and then Tvrtko has added the necessary tools to start using the various tiles. The second patch re-organises the sysfs interface to expose the API for each of the GTs. I decided to prioritise this patch over others to unblock Sujaritha for further development. A third series will still follow this. Andi Shyti (1): drm/i915/gt: make a gt sysfs group and move power management files Tvrtko Ursulin (1): drm/i915: Prepare for multiple GTs drivers/gpu/drm/i915/Makefile | 4 +- drivers/gpu/drm/i915/gt/intel_gt.c| 141 ++- drivers/gpu/drm/i915/gt/intel_gt.h| 14 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 +- drivers/gpu/drm/i915/gt/intel_gt_types.h | 7 + drivers/gpu/drm/i915/gt/sysfs_gt.c| 126 ++ drivers/gpu/drm/i915/gt/sysfs_gt.h| 44 ++ drivers/gpu/drm/i915/gt/sysfs_gt_pm.c | 394 ++ drivers/gpu/drm/i915/gt/sysfs_gt_pm.h | 16 + drivers/gpu/drm/i915/i915_driver.c| 29 +- drivers/gpu/drm/i915/i915_drv.h | 8 + drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/i915_sysfs.c | 315 +- drivers/gpu/drm/i915/i915_sysfs.h | 3 + drivers/gpu/drm/i915/intel_memory_region.h| 3 + drivers/gpu/drm/i915/intel_uncore.c | 12 +- drivers/gpu/drm/i915/intel_uncore.h | 3 +- .../gpu/drm/i915/selftests/mock_gem_device.c | 5 +- 18 files changed, 786 insertions(+), 348 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.c create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.h create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.c create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.h -- 2.34.1
[PATCH 1/2] drm/i915: Prepare for multiple GTs
From: Tvrtko Ursulin
On a multi-tile platform, each tile has its own registers + GGTT
space, and BAR 0 is extended to cover all of them.
Up to four gts are supported in i915->gt[], with slot zero
shadowing the existing i915->gt0 to enable source compatibility
with legacy driver paths. A for_each_gt macro is added to iterate
over the GTs and will be used by upcoming patches that convert
various parts of the driver to be multi-gt aware.
Only the primary/root tile is initialized for now; the other
tiles will be detected and plugged in by future patches once the
necessary infrastructure is in place to handle them.
Signed-off-by: Abdiel Janulgue
Signed-off-by: Daniele Ceraolo Spurio
Signed-off-by: Tvrtko Ursulin
Signed-off-by: Matt Roper
Signed-off-by: Andi Shyti
Cc: Daniele Ceraolo Spurio
Cc: Joonas Lahtinen
Cc: Matthew Auld
---
drivers/gpu/drm/i915/gt/intel_gt.c| 139 --
drivers/gpu/drm/i915/gt/intel_gt.h| 14 +-
drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 +-
drivers/gpu/drm/i915/gt/intel_gt_types.h | 7 +
drivers/gpu/drm/i915/i915_driver.c| 29 ++--
drivers/gpu/drm/i915/i915_drv.h | 6 +
drivers/gpu/drm/i915/intel_memory_region.h| 3 +
drivers/gpu/drm/i915/intel_uncore.c | 12 +-
drivers/gpu/drm/i915/intel_uncore.h | 3 +-
.../gpu/drm/i915/selftests/mock_gem_device.c | 5 +-
10 files changed, 185 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 298ff32c8d0c..5e062c9525f8 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -26,7 +26,8 @@
#include "shmem_utils.h"
#include "pxp/intel_pxp.h"
-void __intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
+static void
+__intel_gt_init_early(struct intel_gt *gt)
{
spin_lock_init(>->irq_lock);
@@ -46,19 +47,27 @@ void __intel_gt_init_early(struct intel_gt *gt, struct
drm_i915_private *i915)
intel_rps_init_early(>->rps);
}
+/* Preliminary initialization of Tile 0 */
void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
{
gt->i915 = i915;
gt->uncore = &i915->uncore;
+
+ __intel_gt_init_early(gt);
}
-int intel_gt_probe_lmem(struct intel_gt *gt)
+static int intel_gt_probe_lmem(struct intel_gt *gt)
{
struct drm_i915_private *i915 = gt->i915;
+ unsigned int instance = gt->info.id;
struct intel_memory_region *mem;
int id;
int err;
+ id = INTEL_REGION_LMEM + instance;
+ if (drm_WARN_ON(&i915->drm, id >= INTEL_REGION_STOLEN_SMEM))
+ return -ENODEV;
+
mem = intel_gt_setup_lmem(gt);
if (mem == ERR_PTR(-ENODEV))
mem = intel_gt_setup_fake_lmem(gt);
@@ -73,9 +82,8 @@ int intel_gt_probe_lmem(struct intel_gt *gt)
return err;
}
- id = INTEL_REGION_LMEM;
-
mem->id = id;
+ mem->instance = instance;
intel_memory_region_set_name(mem, "local%u", mem->instance);
@@ -790,16 +798,21 @@ void intel_gt_driver_release(struct intel_gt *gt)
intel_gt_fini_buffer_pool(gt);
}
-void intel_gt_driver_late_release(struct intel_gt *gt)
+void intel_gt_driver_late_release(struct drm_i915_private *i915)
{
+ struct intel_gt *gt;
+ unsigned int id;
+
/* We need to wait for inflight RCU frees to release their grip */
rcu_barrier();
- intel_uc_driver_late_release(>->uc);
- intel_gt_fini_requests(gt);
- intel_gt_fini_reset(gt);
- intel_gt_fini_timelines(gt);
- intel_engines_free(gt);
+ for_each_gt(gt, i915, id) {
+ intel_uc_driver_late_release(>->uc);
+ intel_gt_fini_requests(gt);
+ intel_gt_fini_reset(gt);
+ intel_gt_fini_timelines(gt);
+ intel_engines_free(gt);
+ }
}
/**
@@ -908,6 +921,112 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt,
i915_reg_t reg)
return intel_uncore_read_fw(gt->uncore, reg);
}
+static int
+intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr)
+{
+ struct drm_i915_private *i915 = gt->i915;
+ unsigned int id = gt->info.id;
+ int ret;
+
+ if (id) {
+ struct intel_uncore_mmio_debug *mmio_debug;
+ struct intel_uncore *uncore;
+
+ /* For multi-tile platforms BAR0 must have at least 16MB per
tile */
+ if (GEM_WARN_ON(pci_resource_len(to_pci_dev(i915->drm.dev), 0) <
+ (id + 1) * SZ_16M))
+ return -EINVAL;
+
+ uncore = kzalloc(sizeof(*uncore), GFP_KERNEL);
+ if (!gt->uncore)
+ return -ENOMEM;
+
+ mmio_debug = kzalloc(sizeof(*mmio_debug), GFP_KERNEL);
+ if (!mmio_debug) {
+ kfree(uncore);
+
[PATCH 2/2] drm/i915/gt: make a gt sysfs group and move power management files
The GT has its own properties and in sysfs they should be grouped
in the 'gt/' directory.
Create a 'gt/' directory in sysfs which will contain gt0...gtN
directories related to each tile configured in the GPU. Move the
power management files inside those directories.
The previous power management files are kept in their original
root directory to avoid breaking the ABI. They point to the tile
'0' and a warning message is printed whenever accessed to. The
deprecated interface needs for the CONFIG_SYSFS_DEPRECATED_V2
flag in order to be generated.
The new sysfs structure will have a similar layout for the 4 tile
case:
/sys/.../card0
├── gt
│ ├── gt0
│ │ ├── id
│ │ ├── rc6_enable
│ │ ├── rc6_residency_ms
│ │ ├── rps_act_freq_mhz
│ │ ├── rps_boost_freq_mhz
│ │ ├── rps_cur_freq_mhz
│ │ ├── rps_max_freq_mhz
│ │ ├── rps_min_freq_mhz
│ │ ├── rps_RP0_freq_mhz
│ │ ├── rps_RP1_freq_mhz
│ │ └── rps_RPn_freq_mhz
. .
. .
. .
│ └── gt3
│ ├── id
│ ├── rc6_enable
│ ├── rc6_residency_ms
│ ├── rps_act_freq_mhz
│ ├── rps_boost_freq_mhz
│ ├── rps_cur_freq_mhz
│ ├── rps_max_freq_mhz
│ ├── rps_min_freq_mhz
│ ├── rps_RP0_freq_mhz
│ ├── rps_RP1_freq_mhz
│ └── rps_RPn_freq_mhz
├── gt_act_freq_mhz -+
├── gt_boost_freq_mhz |
├── gt_cur_freq_mhz|Original interface
├── gt_max_freq_mhz+─-> kept as existing ABI;
├── gt_min_freq_mhz|it points to gt0/
├── gt_RP0_freq_mhz|
└── gt_RP1_freq_mhz|
└── gt_RPn_freq_mhz -+
Signed-off-by: Andi Shyti
Signed-off-by: Lucas De Marchi
Cc: Matt Roper
Cc: Sujaritha Sundaresan
Cc: Tvrtko Ursulin
---
drivers/gpu/drm/i915/Makefile | 4 +-
drivers/gpu/drm/i915/gt/intel_gt.c| 2 +
drivers/gpu/drm/i915/gt/sysfs_gt.c| 126
drivers/gpu/drm/i915/gt/sysfs_gt.h| 44 +++
drivers/gpu/drm/i915/gt/sysfs_gt_pm.c | 394 ++
drivers/gpu/drm/i915/gt/sysfs_gt_pm.h | 16 ++
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/i915_sysfs.c | 315 +---
drivers/gpu/drm/i915/i915_sysfs.h | 3 +
10 files changed, 601 insertions(+), 306 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.c
create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.h
create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.c
create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 1b62b9f65196..0170fdd6f454 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -121,7 +121,9 @@ gt-y += \
gt/intel_timeline.o \
gt/intel_workarounds.o \
gt/shmem_utils.o \
- gt/sysfs_engines.o
+ gt/sysfs_engines.o \
+ gt/sysfs_gt.o \
+ gt/sysfs_gt_pm.o
# autogenerated null render state
gt-y += \
gt/gen6_renderstate.o \
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 5e062c9525f8..cfc0fc127522 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -24,6 +24,7 @@
#include "intel_rps.h"
#include "intel_uncore.h"
#include "shmem_utils.h"
+#include "sysfs_gt.h"
#include "pxp/intel_pxp.h"
static void
@@ -452,6 +453,7 @@ void intel_gt_driver_register(struct intel_gt *gt)
intel_rps_driver_register(>->rps);
intel_gt_debugfs_register(gt);
+ intel_gt_sysfs_register(gt);
}
static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size)
diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.c
b/drivers/gpu/drm/i915/gt/sysfs_gt.c
new file mode 100644
index ..46cf033a53ec
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/sysfs_gt.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "i915_drv.h"
+#include "i915_sysfs.h"
+#include "intel_gt.h"
+#include "intel_gt_types.h"
+#include "intel_rc6.h"
+
+#include "sysfs_gt.h"
+#include "sysfs_gt_pm.h"
+
+struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
+ const char *name)
+{
+ struct kobject *kobj = &dev->kobj;
+
+ /*
+* We are interested at knowing from where the interface
+* has been called, whether it's called from gt/ or from
+* the parent directory.
+* From the interface position it depends also the value of
+* the private data.
+* If the interface is called from gt/ then private data is
+
Re: [PATCH v5 25/32] iommu/mtk: Migrate to aggregate driver
Hi Stephen,
Thanks for helping update here.
On Thu, 2022-01-06 at 13:45 -0800, Stephen Boyd wrote:
> Use an aggregate driver instead of component ops so that we can get
> proper driver probe ordering of the aggregate device with respect to
> all
> the component devices that make up the aggregate device.
>
> Cc: Yong Wu
> Cc: Joerg Roedel
> Cc: Will Deacon
> Cc: Daniel Vetter
> Cc: "Rafael J. Wysocki"
> Cc: Rob Clark
> Cc: Russell King
> Cc: Saravana Kannan
> Signed-off-by: Stephen Boyd
When I test this on mt8195 which have two IOMMU HWs(calling
component_aggregate_regsiter twice), it will abort like this. Then what
should we do if we have two instances?
Thanks.
[2.652424] Error: Driver 'mtk_iommu_agg' is already registered,
aborting...
[2.654033] mtk-iommu: probe of 1c01f000.iommu failed with error -16
[2.662034] Unable to handle kernel NULL pointer dereference at
virtual address 0020
...
[2.672413] pc : aggregate_device_match+0xa8/0x1c8
[2.673027] lr : aggregate_device_match+0x68/0x1c8
...
[2.683091] Call trace:
[2.683403] aggregate_device_match+0xa8/0x1c8
[2.683970] __device_attach_driver+0x38/0xd0
[2.684526] bus_for_each_drv+0x68/0xd0
[2.685015] __device_attach+0xec/0x148
[2.685503] device_attach+0x14/0x20
[2.685960] bus_rescan_devices_helper+0x50/0x90
[2.686545] bus_for_each_dev+0x7c/0xd8
[2.687033] bus_rescan_devices+0x20/0x30
[2.687542] __component_add+0x7c/0xa0
[2.688022] component_add+0x14/0x20
[2.688479] mtk_smi_larb_probe+0xe0/0x120
> ---
> drivers/iommu/mtk_iommu.c| 14 +-
> drivers/iommu/mtk_iommu.h| 6 --
> drivers/iommu/mtk_iommu_v1.c | 14 +-
> 3 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 25b834104790..8e722898cbe2 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -752,9 +752,13 @@ static int mtk_iommu_hw_init(const struct
> mtk_iommu_data *data)
> return 0;
> }
>
> -static const struct component_master_ops mtk_iommu_com_ops = {
> - .bind = mtk_iommu_bind,
> - .unbind = mtk_iommu_unbind,
> +static struct aggregate_driver mtk_iommu_aggregate_driver = {
> + .probe = mtk_iommu_bind,
> + .remove = mtk_iommu_unbind,
> + .driver = {
> + .name = "mtk_iommu_agg",
> + .owner = THIS_MODULE,
> + },
> };
>
> static int mtk_iommu_probe(struct platform_device *pdev)
> @@ -895,7 +899,7 @@ static int mtk_iommu_probe(struct platform_device
> *pdev)
> goto out_list_del;
> }
>
> - ret = component_master_add_with_match(dev, &mtk_iommu_com_ops,
> match);
> + ret = component_aggregate_register(dev,
> &mtk_iommu_aggregate_driver, match);
> if (ret)
> goto out_bus_set_null;
> return ret;
> @@ -928,7 +932,7 @@ static int mtk_iommu_remove(struct
> platform_device *pdev)
> device_link_remove(data->smicomm_dev, &pdev->dev);
> pm_runtime_disable(&pdev->dev);
> devm_free_irq(&pdev->dev, data->irq, data);
> - component_master_del(&pdev->dev, &mtk_iommu_com_ops);
> + component_aggregate_unregister(&pdev->dev,
> &mtk_iommu_aggregate_driver);
> return 0;
> }
>
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index f81fa8862ed0..064fd4f4eade 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -94,15 +94,17 @@ static inline void release_of(struct device *dev,
> void *data)
> of_node_put(data);
> }
>
> -static inline int mtk_iommu_bind(struct device *dev)
> +static inline int mtk_iommu_bind(struct aggregate_device *adev)
> {
> + struct device *dev = adev->parent;
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
>
> return component_bind_all(dev, &data->larb_imu);
> }
>
> -static inline void mtk_iommu_unbind(struct device *dev)
> +static inline void mtk_iommu_unbind(struct aggregate_device *adev)
> {
> + struct device *dev = adev->parent;
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
>
> component_unbind_all(dev, &data->larb_imu);
> diff --git a/drivers/iommu/mtk_iommu_v1.c
> b/drivers/iommu/mtk_iommu_v1.c
> index be22fcf988ce..5fb29058a165 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c
> @@ -534,9 +534,13 @@ static const struct of_device_id
> mtk_iommu_of_ids[] = {
> {}
> };
>
> -static const struct component_master_ops mtk_iommu_com_ops = {
> - .bind = mtk_iommu_bind,
> - .unbind = mtk_iommu_unbind,
> +static struct aggregate_driver mtk_iommu_aggregate_driver = {
> + .probe = mtk_iommu_bind,
> + .remove = mtk_iommu_unbind,
> + .driver = {
> + .name = "mtk_iommu_agg",
> + .owner = THIS_MODULE,
> + },
> };
>
> static int
Re: [PATCH RESEND v4 v5 0/4] drm/vc4: Use the firmware to stop the display pipeline
On Wed, 15 Dec 2021 10:51:13 +0100, Maxime Ripard wrote: > The VC4 driver has had limited support to disable the HDMI controllers and > pixelvalves at boot if the firmware has enabled them. > > However, this proved to be limited, and a bit unreliable so a new firmware > command has been introduced some time ago to make it free all its resources > and > disable any display output it might have enabled. > > [...] Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH RESEND v4 v5 4/4] drm/vc4: Notify the firmware when DRM is in charge
Hi Thomas,
On Tue, Jan 11, 2022 at 10:38:36AM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 15.12.21 um 10:51 schrieb Maxime Ripard:
> > Once the call to drm_fb_helper_remove_conflicting_framebuffers() has
> > been made, simplefb has been unregistered and the KMS driver is entirely
> > in charge of the display.
> >
> > Thus, we can notify the firmware it can free whatever resource it was
> > using to maintain simplefb functional.
> >
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/gpu/drm/vc4/vc4_drv.c | 22 ++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> > index 86c61ee120b7..a03053c8e22c 100644
> > --- a/drivers/gpu/drm/vc4/vc4_drv.c
> > +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> > @@ -37,6 +37,8 @@
> > #include
> > #include
> > +#include
> > +
> > #include "uapi/drm/vc4_drm.h"
> > #include "vc4_drv.h"
> > @@ -215,6 +217,7 @@ static void vc4_match_add_drivers(struct device *dev,
> > static int vc4_drm_bind(struct device *dev)
> > {
> > struct platform_device *pdev = to_platform_device(dev);
> > + struct rpi_firmware *firmware = NULL;
> > struct drm_device *drm;
> > struct vc4_dev *vc4;
> > struct device_node *node;
> > @@ -251,10 +254,29 @@ static int vc4_drm_bind(struct device *dev)
> > if (ret)
> > return ret;
> > + node = of_find_compatible_node(NULL, NULL,
> > "raspberrypi,bcm2835-firmware");
> > + if (node) {
> > + firmware = rpi_firmware_get(node);
> > + of_node_put(node);
> > +
> > + if (!firmware)
> > + return -EPROBE_DEFER;
> > + }
> > +
>
> The code is
>
> Acked-by: Thomas Zimmermann
Thanks for your review
> Just for my understanding:
>
> You retrieve the firmware before killing simpledrm simply to keep the
> display on if it fails, right?
Exactly
> What's the possible error that would justify a retry (via EPROBE_DEFER)?
The firmware there is backed by a driver that might not have probed yet,
in which case we just want to retry later on
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v3 1/2] dt-bindings: display: Turn lvds.yaml into a generic schema
Hi Maxime,
Thank you for the patch.
On Tue, Jan 11, 2022 at 12:06:34PM +0100, Maxime Ripard wrote:
> The lvds.yaml file so far was both defining the generic LVDS properties
> (such as data-mapping) that could be used for any LVDS sink, but also
> the panel-lvds binding.
>
> That last binding was to describe LVDS panels simple enough, and had a
> number of other bindings using it as a base to specialise it further.
>
> However, this situation makes it fairly hard to extend and reuse both
> the generic parts, and the panel-lvds itself.
>
> Let's remove the panel-lvds parts and leave only the generic LVDS
> properties.
>
> Reviewed-by: Rob Herring
> Signed-off-by: Maxime Ripard
>
> ---
>
> Changes from v2:
> - Fix references to that file
>
> Changes from v1:
> - Moved the schema out of panel
> ---
> .../bindings/display/bridge/lvds-codec.yaml | 2 +-
> .../bindings/display/{panel => }/lvds.yaml| 31 ++-
> .../display/panel/advantech,idk-1110wr.yaml | 19 ++--
> .../display/panel/innolux,ee101ia-01d.yaml| 23 --
> .../display/panel/mitsubishi,aa104xd12.yaml | 19 ++--
> .../display/panel/mitsubishi,aa121td01.yaml | 19 ++--
> .../display/panel/sgd,gktw70sdae4se.yaml | 19 ++--
> MAINTAINERS | 2 +-
> 8 files changed, 93 insertions(+), 41 deletions(-)
> rename Documentation/devicetree/bindings/display/{panel => }/lvds.yaml (86%)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> index 5079c1cc337b..27b905b81b12 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> @@ -67,7 +67,7 @@ properties:
>- vesa-24
> description: |
>The color signals mapping order. See details in
> - Documentation/devicetree/bindings/display/panel/lvds.yaml
> + Documentation/devicetree/bindings/display/lvds.yaml
>
>port@1:
> $ref: /schemas/graph.yaml#/properties/port
> diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> b/Documentation/devicetree/bindings/display/lvds.yaml
> similarity index 86%
> rename from Documentation/devicetree/bindings/display/panel/lvds.yaml
> rename to Documentation/devicetree/bindings/display/lvds.yaml
> index 49460c9dceea..55751402fb13 100644
> --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> +++ b/Documentation/devicetree/bindings/display/lvds.yaml
> @@ -1,10 +1,10 @@
> # SPDX-License-Identifier: GPL-2.0
> %YAML 1.2
> ---
> -$id: http://devicetree.org/schemas/display/panel/lvds.yaml#
> +$id: http://devicetree.org/schemas/display/lvds.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: LVDS Display Panel
> +title: LVDS Display Common Properties
>
> maintainers:
>- Laurent Pinchart
> @@ -26,18 +26,7 @@ description: |+
The description mentions "This bindings supports display panels
compatible with the following specifications". This needs a small update
to avoid referring to panels.
With this updated,
Reviewed-by: Laurent Pinchart
>Device compatible with those specifications have been marketed under the
>FPD-Link and FlatLink brands.
>
> -allOf:
> - - $ref: panel-common.yaml#
> -
> properties:
> - compatible:
> -contains:
> - const: panel-lvds
> -description:
> - Shall contain "panel-lvds" in addition to a mandatory panel-specific
> - compatible string defined in individual panel bindings. The
> "panel-lvds"
> - value shall never be used on its own.
> -
>data-mapping:
> enum:
>- jeida-18
> @@ -96,22 +85,6 @@ properties:
>If set, reverse the bit order described in the data mappings below on
> all
>data lanes, transmitting bits for slots 6 to 0 instead of 0 to 6.
>
> - port: true
> - ports: true
> -
> -required:
> - - compatible
> - - data-mapping
> - - width-mm
> - - height-mm
> - - panel-timing
> -
> -oneOf:
> - - required:
> - - port
> - - required:
> - - ports
> -
> additionalProperties: true
>
> ...
> diff --git
> a/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml
> b/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml
> index 93878c2cd370..3a8c2c11f9bd 100644
> ---
> a/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml
> +++
> b/Documentation/devicetree/bindings/display/panel/advantech,idk-1110wr.yaml
> @@ -11,13 +11,23 @@ maintainers:
>- Thierry Reding
>
> allOf:
> - - $ref: lvds.yaml#
> + - $ref: panel-common.yaml#
> + - $ref: /schemas/display/lvds.yaml/#
> +
> +select:
> + properties:
> +compatible:
> + contains:
> +const: advantech,idk-1110wr
> +
> + required:
> +- compatible
>
Re: [PATCH v3 2/2] dt-bindings: panel: Introduce a panel-lvds binding
Hi Maxime, Thank you for the patch. On Tue, Jan 11, 2022 at 12:06:35PM +0100, Maxime Ripard wrote: > Following the previous patch, let's introduce a generic panel-lvds > binding that documents the panels that don't have any particular > constraint documented. > > Reviewed-by: Rob Herring > Signed-off-by: Maxime Ripard > > --- > > Changes from v2: > - Added a MAINTAINERS entry > > Changes from v1: > - Added missing compatible > - Fixed lint > --- > .../bindings/display/panel/panel-lvds.yaml| 57 +++ > MAINTAINERS | 1 + > 2 files changed, 58 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-lvds.yaml > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml > b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml > new file mode 100644 > index ..fcc50db6a812 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.yaml > @@ -0,0 +1,57 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/panel/panel-lvds.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Generic LVDS Display Panel Device Tree Bindings > + > +maintainers: > + - Lad Prabhakar > + - Thierry Reding > + > +allOf: > + - $ref: panel-common.yaml# > + - $ref: /schemas/display/lvds.yaml/# > + > +select: > + properties: > +compatible: > + contains: > +const: panel-lvds > + > + not: > +properties: > + compatible: > +contains: > + enum: > +- advantech,idk-1110wr > +- advantech,idk-2121wr > +- innolux,ee101ia-01d > +- mitsubishi,aa104xd12 > +- mitsubishi,aa121td01 > +- sgd,gktw70sdae4se I still don't like this :-( Couldn't we instead do select: properties: compatible: contains: enum: - auo,b101ew05 - tbs,a711-panel ? > + > + required: > +- compatible > + > +properties: > + compatible: > +items: > + - enum: > + - auo,b101ew05 > + - tbs,a711-panel > + > + - const: panel-lvds > + > +unevaluatedProperties: false > + > +required: > + - compatible > + - data-mapping > + - width-mm > + - height-mm > + - panel-timing > + - port > + > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 368072da0a05..02001455949e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6080,6 +6080,7 @@ T: git git://anongit.freedesktop.org/drm/drm-misc > S: Maintained > F: drivers/gpu/drm/panel/panel-lvds.c > F: Documentation/devicetree/bindings/display/lvds.yaml > +F: Documentation/devicetree/bindings/display/panel/panel-lvds.yaml > > DRM DRIVER FOR MANTIX MLAF057WE51 PANELS > M: Guido Günther -- Regards, Laurent Pinchart
Re: [PATCH v2 0/2] video: A couple of fixes for the vga16fb driver
Hi,
On Mon, Jan 10, 2022 at 10:56:23AM +0100, Javier Martinez Canillas wrote:
> This patch series contains two fixes for the vga16fb driver. I looked at
> the driver due a regression reported [0], caused by commit d391c5827107
> ("drivers/firmware: move x86 Generic System Framebuffers support").
>
> The mentioned commit didn't change any logic but just moved the platform
> device registration that matches the vesafb and efifb drivers to happen
> later. And this caused the vga16fb driver to be probed even in machines
> that don't have an EGA or VGA video adapter.
>
> This is a v2 of the patch series that addresses issues pointed out by
> Geert Uytterhoeven.
>
> Patch #1 is fixing the wrong check to determine if either EGA or VGA is
> used and patch #2 adds a check to the driver to only be loaded for EGA
> and VGA 16 color graphic cards.
For both patches,
Acked-by: Maxime Ripard
Maxime
signature.asc
Description: PGP signature
[PATCH] drm/mipi-dbi: Fix source-buffer address in mipi_dbi_buf_copy
Set the source-buffer address after mapping the buffer into the
kernel's address space. Makes MIPI DBI helpers work again.
Signed-off-by: Thomas Zimmermann
Fixes: c47160d8edcd ("drm/mipi-dbi: Remove dependency on GEM CMA helper
library")
Reported-by: Noralf Trønnes
Cc: Thomas Zimmermann
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Maxime Ripard
---
drivers/gpu/drm/drm_mipi_dbi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index ded8968b3e8a..0327d595e028 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -209,11 +209,11 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer
*fb,
ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
if (ret)
return ret;
- src = data[0].vaddr; /* TODO: Use mapping abstraction properly */
ret = drm_gem_fb_vmap(fb, map, data);
if (ret)
goto out_drm_gem_fb_end_cpu_access;
+ src = data[0].vaddr; /* TODO: Use mapping abstraction properly */
switch (fb->format->format) {
case DRM_FORMAT_RGB565:
--
2.34.1
Re: [PATCH v2] drm/bridge: analogix_dp: Grab runtime PM reference for DP-AUX
Hi Brian,
I am not DP specialist so CC-ed people working with DP
On 01.10.2021 23:42, Brian Norris wrote:
If the display is not enable()d, then we aren't holding a runtime PM
reference here. Thus, it's easy to accidentally cause a hang, if user
space is poking around at /dev/drm_dp_aux0 at the "wrong" time.
Let's get the panel and PM state right before trying to talk AUX.
Fixes: 0d97ad03f422 ("drm/bridge: analogix_dp: Remove duplicated code")
Cc:
Cc: Tomeu Vizoso
Signed-off-by: Brian Norris
Few questions/issues here:
1. If it is just to avoid accidental 'hangs' it would be better to just
check if the panel is working before transfer, if not, return error
code. If there is better reason for this pm dance, please provide it in
description.
2. Again I see an assumption that panel-prepare enables power for
something different than video transmission, accidentally it is true for
most devices, but devices having more fine grained power management will
break, or at least will be used inefficiently - but maybe in case of dp
it is OK ???
3. More general issue - I am not sure if this should not be handled
uniformly for all drm_dp devices.
Regards
Andrzej
---
Changes in v2:
- Fix spelling in Subject
- DRM_DEV_ERROR() -> drm_err()
- Propagate errors from un-analogix_dp_prepare_panel()
.../drm/bridge/analogix/analogix_dp_core.c| 21 ++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index b7d2e4449cfa..6fc46ac93ef8 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1632,8 +1632,27 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux
*aux,
struct drm_dp_aux_msg *msg)
{
struct analogix_dp_device *dp = to_dp(aux);
+ int ret, ret2;
- return analogix_dp_transfer(dp, msg);
+ ret = analogix_dp_prepare_panel(dp, true, false);
+ if (ret) {
+ drm_err(dp->drm_dev, "Failed to prepare panel (%d)\n", ret);
+ return ret;
+ }
+
+ pm_runtime_get_sync(dp->dev);
+ ret = analogix_dp_transfer(dp, msg);
+ pm_runtime_put(dp->dev);
+
+ ret2 = analogix_dp_prepare_panel(dp, false, false);
+ if (ret2) {
+ drm_err(dp->drm_dev, "Failed to unprepare panel (%d)\n", ret2);
+ /* Prefer the analogix_dp_transfer() error, if it exists. */
+ if (!ret)
+ ret = ret2;
+ }
+
+ return ret;
}
struct analogix_dp_device *
Re: [PATCH] drm/mipi-dbi: Fix source-buffer address in mipi_dbi_buf_copy
Den 11.01.2022 14.26, skrev Thomas Zimmermann:
> Set the source-buffer address after mapping the buffer into the
> kernel's address space. Makes MIPI DBI helpers work again.
>
> Signed-off-by: Thomas Zimmermann
> Fixes: c47160d8edcd ("drm/mipi-dbi: Remove dependency on GEM CMA helper
> library")
> Reported-by: Noralf Trønnes
> Cc: Thomas Zimmermann
> Cc: Daniel Vetter
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> ---
Reviewed-by: Noralf Trønnes
Re: [PATCH v4 0/6] drm: exynos: dsi: Convert drm bridge
On Tue, Jan 11, 2022 at 5:20 PM Andrzej Hajda wrote:
>
> Hi Jagan,
>
> On 11.01.2022 10:32, Jagan Teki wrote:
> > Hi Andrzej,
> >
> > On Tue, Dec 28, 2021 at 4:18 PM Andrzej Hajda
> > wrote:
> >> Hi Marek,
> >>
> >> On 23.12.2021 10:15, Marek Szyprowski wrote:
> >>> Hi Jagan,
> >>>
> >>> On 18.12.2021 00:16, Marek Szyprowski wrote:
> On 15.12.2021 15:56, Jagan Teki wrote:
> > On Wed, Dec 15, 2021 at 7:49 PM Marek Szyprowski
> > wrote:
> >> On 15.12.2021 13:57, Jagan Teki wrote:
> >>> On Wed, Dec 15, 2021 at 5:31 PM Marek Szyprowski
> >>> wrote:
> On 15.12.2021 11:15, Jagan Teki wrote:
> > Updated series about drm bridge conversion of exynos dsi.
> > Previous version can be accessible, here [1].
> >
> > Patch 1: connector reset
> >
> > Patch 2: panel_bridge API
> >
> > Patch 3: Bridge conversion
> >
> > Patch 4: Atomic functions
> >
> > Patch 5: atomic_set
> >
> > Patch 6: DSI init in enable
> There is a little progress! :)
>
> Devices with a simple display pipeline (only a DSI panel, like
> Trats/Trats2) works till the last patch. Then, after applying
> ("[PATCH
> v4 6/6] drm: exynos: dsi: Move DSI init in bridge enable"), I get no
> display at all.
>
> A TM2e board with in-bridge (Exynos MIC) stops displaying anything
> after
> applying patch "[PATCH v4 2/6] drm: exynos: dsi: Use drm
> panel_bridge API".
>
> In case of the Arndale board with tc358764 bridge, no much
> progress. The
> display is broken just after applying the "[PATCH v2] drm: bridge:
> tc358764: Use drm panel_bridge API" patch on top of linux-next.
>
> In all cases the I had "drm: of: Lookup if child node has panel or
> bridge" patch applied.
> >>> Just skip the 6/6 for now.
> >>>
> >>> Apply
> >>> -
> >>> https://protect2.fireeye.com/v1/url?k=a24f3f76-fdd40659-a24eb439-0cc47a31cdf8-97ea12b4c5258d11&q=1&e=37a169bf-7ca5-4362-aad7-486018c7a708&u=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fpatch%2F1825%2F
> >>> -
> >>> https://protect2.fireeye.com/v1/url?k=a226360f-fdbd0f20-a227bd40-0cc47a31cdf8-ebd66aebee1058d7&q=1&e=37a169bf-7ca5-4362-aad7-486018c7a708&u=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fpatch%2F1823%2F
> >>>
> >>> Then apply 1/6 to 5/6. and update the status?
> >> Okay, my fault, I didn't check that case on Arndale.
> >>
> >> I've checked and indeed, Trats/Trats2 and Arndale works after the above
> >> 2 patches AND patches 1-5.
> >>
> >> The only problem is now on TM2e, which uses Exynos MIC as in-bridge for
> >> Exynos DSI:
> >>
> >> [4.068866] [drm] Exynos DRM: using 1380.decon device for DMA
> >> mapping operations
> >> [4.069183] exynos-drm exynos-drm: bound 1380.decon (ops
> >> decon_component_ops)
> >> [4.128983] exynos-drm exynos-drm: bound 1388.decon (ops
> >> decon_component_ops)
> >> [4.129261] exynos-drm exynos-drm: bound 1393.mic (ops
> >> exynos_mic_component_ops)
> >> [4.133508] exynos-dsi 1390.dsi: [drm:exynos_dsi_host_attach]
> >> *ERROR* failed to find the bridge: -19
> >> [4.136392] exynos-drm exynos-drm: bound 1390.dsi (ops
> >> exynos_dsi_component_ops)
> >> [4.145499] rc_core: Couldn't load IR keymap rc-cec
> >> [4.145666] Registered IR keymap rc-empty
> >> [4.148402] rc rc0: sii8620 as /devices/virtual/rc/rc0
> >> [4.156051] input: sii8620 as /devices/virtual/rc/rc0/input1
> >> [4.160647] exynos-drm exynos-drm: bound 1397.hdmi (ops
> >> hdmi_component_ops)
> >> [4.169923] exynos-drm exynos-drm: [drm] Cannot find any crtc or
> >> sizes
> >> [4.173958] exynos-drm exynos-drm: [drm] Cannot find any crtc or
> >> sizes
> >> [4.182304] [drm] Initialized exynos 1.1.0 20180330 for
> >> exynos-drm on
> >> minor 0
> >>
> >> The display pipeline for TM2e is:
> >>
> >> Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel
> > If Trats/Trats2 is working then it has to work. I don't see any
> > difference in output pipeline. Can you please share the full log, I
> > cannot see host_attach print saying "Attached.."
> Well, there is a failure message about the panel:
>
> exynos-dsi 1390.dsi: [drm:exynos_dsi_host_attach] *ERROR* failed
> to find the bridge: -19
>
> however it looks that something might be broken in dts. The in-bridge
> (Exynos MIC) is on port 0 and the panel is @0, what imho might cause
> the issue.
>
> I've tried to change in in-bridge ('mic_to_dsi') port to 1 in
> exynos5433.dtsi. Then the panel has been attached:
>
> exyn
Re: Phyr Starter
On Tue, Jan 11, 2022 at 12:40:10PM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 10.01.22 um 20:34 schrieb Matthew Wilcox:
> > TLDR: I want to introduce a new data type:
> >
> > struct phyr {
> > phys_addr_t addr;
> > size_t len;
> > };
>
> Did you look at struct dma_buf_map? [1]
Thanks. I wasn't aware of that. It doesn't seem to actually solve the
problem, in that it doesn't carry any length information. Did you mean
to point me at a different structure?
Re: Phyr Starter
On Tue, Jan 11, 2022 at 12:17:18AM -0800, John Hubbard wrote: > Zooming in on the pinning aspect for a moment: last time I attempted to > convert O_DIRECT callers from gup to pup, I recall wanting very much to > record, in each bio_vec, whether these pages were acquired via FOLL_PIN, > or some non-FOLL_PIN method. Because at the end of the IO, it is not > easy to disentangle which pages require put_page() and which require > unpin_user_page*(). > > And changing the bio_vec for *that* purpose was not really acceptable. > > But now that you're looking to change it in a big way (and with some > spare bits avaiable...oohh!), maybe I can go that direction after all. > > Or, are you looking at a design in which any phyr is implicitly FOLL_PIN'd > if it exists at all? That. I think there's still good reasons to keep a single-page (or maybe dual-page) GUP around, but no reason to mix it with ranges. > Or any other thoughts in this area are very welcome. That's there's no support for unpinning part of a range. You pin it, do the IO, unpin it. That simplifies the accounting.
Re: Phyr Starter
Hi
Am 11.01.22 um 14:56 schrieb Matthew Wilcox:
On Tue, Jan 11, 2022 at 12:40:10PM +0100, Thomas Zimmermann wrote:
Hi
Am 10.01.22 um 20:34 schrieb Matthew Wilcox:
TLDR: I want to introduce a new data type:
struct phyr {
phys_addr_t addr;
size_t len;
};
Did you look at struct dma_buf_map? [1]
Thanks. I wasn't aware of that. It doesn't seem to actually solve the
problem, in that it doesn't carry any length information. Did you mean
to point me at a different structure?
It's the structure I meant. It refers to a buffer, so the length could
be added. For something more sophisticated, dma_buf_map could be changed
to distinguish between the buffer and an iterator pointing into the buffer.
But if it's really different, then so be it.
Best regards
Thomas
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: Phyr Starter
On Tue, Jan 11, 2022 at 04:32:56AM +, Matthew Wilcox wrote:
> On Mon, Jan 10, 2022 at 08:41:26PM -0400, Jason Gunthorpe wrote:
> > On Mon, Jan 10, 2022 at 07:34:49PM +, Matthew Wilcox wrote:
> >
> > > Finally, it may be possible to stop using scatterlist to describe the
> > > input to the DMA-mapping operation. We may be able to get struct
> > > scatterlist down to just dma_address and dma_length, with chaining
> > > handled through an enclosing struct.
> >
> > Can you talk about this some more? IMHO one of the key properties of
> > the scatterlist is that it can hold huge amounts of pages without
> > having to do any kind of special allocation due to the chaining.
> >
> > The same will be true of the phyr idea right?
>
> My thinking is that we'd pass a relatively small array of phyr (maybe 16
> entries) to get_user_phyr(). If that turned out not to be big enough,
> then we have two options; one is to map those 16 ranges with sg and use
> the sg chaining functionality before throwing away the phyr and calling
> get_user_phyr() again.
Then we are we using get_user_phyr() at all if we are just storing it
in a sg?
Also 16 entries is way to small, it should be at least a whole PMD
worth so we don't have to relock the PMD level each iteration.
I would like to see a flow more like:
cpu_phyr_list = get_user_phyr(uptr, 1G);
dma_phyr_list = dma_map_phyr(device, cpu_phyr_list);
[..]
dma_unmap_phyr(device, dma_phyr_list);
unpin_drity_free(cpu_phy_list);
Where dma_map_phyr() can build a temporary SGL for old iommu drivers
compatability. iommu drivers would want to implement natively, of
course.
ie no loops in drivers.
> The question is whether this is the right kind of optimisation to be
> doing. I hear you that we want a dense format, but it's questionable
> whether the kind of thing you're suggesting is actually denser than this
> scheme. For example, if we have 1GB pages and userspace happens to have
> allocated pages (3, 4, 5, 6, 7, 8, 9, 10) then this can be represented
> as a single phyr. A power-of-two scheme would have us use four entries
> (3, 4-7, 8-9, 10).
That is not quite what I had in mind..
struct phyr_list {
unsigned int first_page_offset_bytes;
size_t total_length_bytes;
phys_addr_t min_alignment;
struct packed_phyr *list_of_pages;
};
Where each 'packed_phyr' is an aligned page of some kind. The packing
has to be able to represent any number of pfns, so we have four major
cases:
- 4k pfns (use 8 bytes)
- Natural order pfn (use 8 bytes)
- 4k aligned pfns, arbitary number (use 12 bytes)
- <4k aligned, arbitary length (use 16 bytes?)
In all cases the interior pages are fully used, only the first and
last page is sliced based on the two parameters in the phyr_list.
The first_page_offset_bytes/total_length_bytes mean we don't need to
use the inefficient coding for many common cases, just stick to the 4k
coding and slice the first/last page down.
The last case is, perhaps, a possible route to completely replace
scatterlist. Few places need true byte granularity for interior pages,
so we can invent some coding to say 'this is 8 byte aligned, and n
bytes long' that only fits < 4k or something. Exceptional cases can
then still work. I'm not sure what block needs here - is it just 512?
Basically think of list_of_pages as showing a contiguous list of at
least min_aligned pages and first_page_offset_bytes/total_length_bytes
taking a byte granular slice out of that logical range.
>From a HW perspective I see two basic modalities:
- Streaming HW, which read/writes in a single pass (think
NVMe/storage/network). Usually takes a list of dma_addr_t and
length that HW just walks over. Rarely cares about things like page
boundaries. Optimization goal is to minimize list length. In this
case we map each packed_phyr into a HW SGL
- Random Access HW, which is randomly touching memory (think RDMA,
VFIO, DRM, IOMMU). Usually stores either a linear list of same-size
dma_addr_t pages, or a radix tree page table of dma_addr_t.
Needs to have a minimum alignment of each chunk (usually 4k) to
represent it. Optimization goal is to have maximum page size. In
this case we use min_alignment to size the HW array and decode the
packed_phyrs into individual pages.
> Using a (dma_addr, size_t) tuple makes coalescing adjacent pages very
> cheap.
With the above this still works, the very last entry in list_of_pages
would be the 12 byte pfn type and when we start a new page the logic
would then optimize it down to 8 bytes, if possible. At that point we
know we are not going to change it:
- An interior page that is up perfectly aligned is represented as a
natural order
- A starting page that ends on perfect alignment is widened to
natural order and first_page_offset_bytes is corrected
- An ending page that starts on perfect alignment is widened to
natural order and total_length_bytes is set
(though no harm in keeping the 12 byte repr
Re: Phyr Starter
On Tue, Jan 11, 2022 at 02:01:17PM +, Matthew Wilcox wrote: > On Tue, Jan 11, 2022 at 12:17:18AM -0800, John Hubbard wrote: > > Zooming in on the pinning aspect for a moment: last time I attempted to > > convert O_DIRECT callers from gup to pup, I recall wanting very much to > > record, in each bio_vec, whether these pages were acquired via FOLL_PIN, > > or some non-FOLL_PIN method. Because at the end of the IO, it is not > > easy to disentangle which pages require put_page() and which require > > unpin_user_page*(). > > > > And changing the bio_vec for *that* purpose was not really acceptable. > > > > But now that you're looking to change it in a big way (and with some > > spare bits avaiable...oohh!), maybe I can go that direction after all. > > > > Or, are you looking at a design in which any phyr is implicitly FOLL_PIN'd > > if it exists at all? > > That. I think there's still good reasons to keep a single-page (or > maybe dual-page) GUP around, but no reason to mix it with ranges. > > > Or any other thoughts in this area are very welcome. > > That's there's no support for unpinning part of a range. You pin it, > do the IO, unpin it. That simplifies the accounting. VFIO wouldn't like this :( Jason
Re: [git pull] drm for 5.17-rc1 (pre-merge window pull)
On Mon, Jan 10, 2022 at 9:53 PM Linus Torvalds wrote: > > On Mon, Jan 10, 2022 at 6:44 PM Linus Torvalds > wrote: > > > > I'll double-check to see if a revert fixes it at the top of my tree. > > Yup. It reverts cleanly, and the end result builds and works fine, and > doesn't show the horrendous flickering. > > I have done that revert, and will continue the merge window work. > Somebody else gets to figure out what the actual bug is, but that > commit was horribly broken on my machine (Sapphire Pulse RX 580 8GB, > fwiw). Thanks for tracking this down. We are investigating the issue. Alex
Re: [git pull] drm for 5.17-rc1 (pre-merge window pull)
On 2022-01-11 10:08, Alex Deucher wrote:
> On Mon, Jan 10, 2022 at 9:53 PM Linus Torvalds
> wrote:
>>
>> On Mon, Jan 10, 2022 at 6:44 PM Linus Torvalds
>> wrote:
>>>
>>> I'll double-check to see if a revert fixes it at the top of my tree.
>>
>> Yup. It reverts cleanly, and the end result builds and works fine, and
>> doesn't show the horrendous flickering.
>>
>> I have done that revert, and will continue the merge window work.
>> Somebody else gets to figure out what the actual bug is, but that
>> commit was horribly broken on my machine (Sapphire Pulse RX 580 8GB,
>> fwiw).
>
> Thanks for tracking this down. We are investigating the issue.
>
Thanks for tracking this down. It was the result of a bad merge
from an internal branch.
Attached is a v2 of the buggy patch that should get this right.
If you have a chance to try it out let us know, if not we'll get
someone to repro and test the fix.
Harry
> Alex
From 5ed98e330781615434711a5fc31a6a7473f9344f Mon Sep 17 00:00:00 2001
From: Meenakshikumar Somasundaram
Date: Mon, 15 Nov 2021 01:51:37 -0500
Subject: [PATCH] drm/amd/display: Fix for otg synchronization logic
[Why]
During otg sync trigger, plane states are used to decide whether the otg
is already synchronized or not. There are scenarions when otgs are
disabled without plane state getting disabled and in such case the otg is
excluded from synchronization.
[How]
Introduced pipe_idx_syncd in pipe_ctx that tracks each otgs master pipe.
When a otg is disabled/enabled, pipe_idx_syncd is reset to itself.
On sync trigger, pipe_idx_syncd is checked to decide whether a otg is
already synchronized and the otg is further included or excluded from
synchronization.
v2:
Don't drop is_blanked logic
Reviewed-by: Jun Lei
Reviewed-by: Mustapha Ghaddar
Acked-by: Bhawanpreet Lakha
Signed-off-by: meenakshikumar somasundaram
Tested-by: Daniel Wheeler
Signed-off-by: Alex Deucher
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 40 +-
.../gpu/drm/amd/display/dc/core/dc_resource.c | 54 +++
drivers/gpu/drm/amd/display/dc/dc.h | 1 +
.../display/dc/dce110/dce110_hw_sequencer.c | 8 +++
.../drm/amd/display/dc/dcn31/dcn31_resource.c | 3 ++
.../gpu/drm/amd/display/dc/inc/core_types.h | 1 +
drivers/gpu/drm/amd/display/dc/inc/resource.h | 11
7 files changed, 105 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 01c8849b9db2..6f5528d34093 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1404,20 +1404,34 @@ static void program_timing_sync(
status->timing_sync_info.master = false;
}
- /* remove any other unblanked pipes as they have already been synced */
- for (j = j + 1; j < group_size; j++) {
- bool is_blanked;
- if (pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked)
-is_blanked =
- pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked(pipe_set[j]->stream_res.opp);
- else
-is_blanked =
- pipe_set[j]->stream_res.tg->funcs->is_blanked(pipe_set[j]->stream_res.tg);
- if (!is_blanked) {
-group_size--;
-pipe_set[j] = pipe_set[group_size];
-j--;
+ /* remove any other pipes that are already been synced */
+ if (dc->config.use_pipe_ctx_sync_logic) {
+ /* check pipe's syncd to decide which pipe to be removed */
+ for (j = 1; j < group_size; j++) {
+if (pipe_set[j]->pipe_idx_syncd == pipe_set[0]->pipe_idx_syncd) {
+ group_size--;
+ pipe_set[j] = pipe_set[group_size];
+ j--;
+} else
+ /* link slave pipe's syncd with master pipe */
+ pipe_set[j]->pipe_idx_syncd = pipe_set[0]->pipe_idx_syncd;
+ }
+ } else {
+ for (j = j + 1; j < group_size; j++) {
+bool is_blanked;
+
+if (pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked)
+ is_blanked =
+ pipe_set[j]->stream_res.opp->funcs->dpg_is_blanked(pipe_set[j]->stream_res.opp);
+else
+ is_blanked =
+ pipe_set[j]->stream_res.tg->funcs->is_blanked(pipe_set[j]->stream_res.tg);
+if (!is_blanked) {
+ group_size--;
+ pipe_set[j] = pipe_set[group_size];
+ j--;
+}
}
}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index de5c7d1e0267..eaeef72773f6 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -3216,3 +3216,57 @@ struct hpo_dp_link_encoder *resource_get_hpo_dp_link_enc_for_det_lt(
return hpo_dp_link_enc;
}
#endif
+
+void reset_syncd_pipes_from_disabled_pipes(struct dc *dc,
+ struct dc_state *context)
+{
+ int i, j;
+ struct pipe_ctx *pipe_ctx_old, *pipe_ctx, *pipe_ctx_syncd;
+
+ /* If pipe backend is reset, need to reset pipe syncd status */
+ for (i = 0; i < dc->res_pool->pipe_count; i++) {
+ pipe_ctx_old = &dc->current_state->res_ctx.pipe_ctx[i];
+ pipe_ctx = &context->res_ct
Re: [PATCH] drm/atomic: Check new_crtc_state->active to determine if CRTC needs disable in self refresh mode
Pushed out to drm-misc-next-fixes.
Alex
On Fri, Jan 7, 2022 at 9:07 PM Liu Ying wrote:
>
> On Fri, 2022-01-07 at 14:53 -0500, Alex Deucher wrote:
> > On Wed, Dec 29, 2021 at 11:07 PM Liu Ying wrote:
> > >
> > > Actual hardware state of CRTC is controlled by the member 'active'
> > > in
> > > struct drm_crtc_state instead of the member 'enable', according to
> > > the
> > > kernel doc of the member 'enable'. In fact, the drm client modeset
> > > and atomic helpers are using the member 'active' to do the control.
> > >
> > > Referencing the member 'enable' of new_crtc_state, the function
> > > crtc_needs_disable() may fail to reflect if CRTC needs disable in
> > > self refresh mode, e.g., when the framebuffer emulation will be
> > > blanked
> > > through the client modeset helper with the next commit, the member
> > > 'enable' of new_crtc_state is still true while the member 'active'
> > > is
> > > false, hence the relevant potential encoder and bridges won't be
> > > disabled.
> > >
> > > So, let's check new_crtc_state->active to determine if CRTC needs
> > > disable
> > > in self refresh mode instead of new_crtc_state->enable.
> > >
> > > Fixes: 1452c25b0e60 ("drm: Add helpers to kick off self refresh
> > > mode in drivers")
> > > Cc: Sean Paul
> > > Cc: Rob Clark
> > > Cc: Maarten Lankhorst
> > > Cc: Maxime Ripard
> > > Cc: Thomas Zimmermann
> > > Cc: David Airlie
> > > Cc: Daniel Vetter
> > > Signed-off-by: Liu Ying
> >
> > Reviewed-by: Alex Deucher
> >
> > Do you need someone to push this for you?
>
> Yes, please. Thanks.
>
> Liu Ying
>
> >
> > Alex
> >
> > > ---
> > > drivers/gpu/drm/drm_atomic_helper.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index a7a05e1e26bb..9603193d2fa1 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -1016,7 +1016,7 @@ crtc_needs_disable(struct drm_crtc_state
> > > *old_state,
> > > * it's in self refresh mode and needs to be fully
> > > disabled.
> > > */
> > > return old_state->active ||
> > > - (old_state->self_refresh_active && !new_state-
> > > >enable) ||
> > > + (old_state->self_refresh_active && !new_state-
> > > >active) ||
> > >new_state->self_refresh_active;
> > > }
> > >
> > > --
> > > 2.25.1
> > >
>
Re: [Patch v4 18/24] drm/amdkfd: CRIU checkpoint and restore xnack mode
On 2022-01-10 7:10 p.m., Felix Kuehling
wrote:
On
2022-01-05 10:22 a.m., philip yang wrote:
On 2021-12-22 7:37 p.m., Rajneesh Bhardwaj wrote:
Recoverable page faults are represented
by the xnack mode setting inside
a kfd process and are used to represent the device page
faults. For CR,
we don't consider negative values which are typically used for
querying
the current xnack mode without modifying it.
Signed-off-by: Rajneesh
Bhardwaj
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 15
+++
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 178b0ccfb286..446eb9310915 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1845,6 +1845,11 @@ static int
criu_checkpoint_process(struct kfd_process *p,
memset(&process_priv, 0, sizeof(process_priv));
process_priv.version = KFD_CRIU_PRIV_VERSION;
+ /* For CR, we don't consider negative xnack mode which is
used for
+ * querying without changing it, here 0 simply means
disabled and 1
+ * means enabled so retry for finding a valid PTE.
+ */
Negative value to query xnack mode is for
kfd_ioctl_set_xnack_mode user space ioctl interface, which is
not used by CRIU, I think this comment is misleading,
+ process_priv.xnack_mode =
p->xnack_enabled ? 1 : 0;
change to process_priv.xnack_enabled
ret =
copy_to_user(user_priv_data + *priv_offset,
&process_priv, sizeof(process_priv));
@@ -2231,6 +2236,16 @@ static int criu_restore_process(struct
kfd_process *p,
return -EINVAL;
}
+ pr_debug("Setting XNACK mode\n");
+ if (process_priv.xnack_mode &&
!kfd_process_xnack_mode(p, true)) {
+ pr_err("xnack mode cannot be set\n");
+ ret = -EPERM;
+ goto exit;
+ } else {
On GFXv9 GPUs except Aldebaran, this means the process
checkpointed is xnack off, it can restore and resume on GPU with
xnack on, then shader will continue running successfully, but
driver is not guaranteed to map svm ranges on GPU all the time,
if retry fault happens, the shader will not recover. Maybe
change to:
If (KFD_GC_VERSION(dev) != IP_VERSION(9, 4, 2) {
The code here was correct. The xnack mode applies to the whole
process, not just one GPU. The logic for checking the capabilities
of all GPUs is already in kfd_process_xnack_mode. If XNACK cannot
be supported by all GPUs, restoring a non-0 XNACK mode will fail.
Any GPU can run in XNACK-disabled mode. So we don't need any
limitations for process_priv.xnack_enabled == 0.
Yes, the code was correct, for case all GPUs dev->noretry=0
(xnack on), process->xnack_enabled=0, we unmap the queues while
migrating, guarantee to map svm ranges on GPUs then resume queues.
If retry fault happens, we don't recover the fault, report the
fault to user space. That is all correct.
Regards,
Philip
Regards,
Felix
if (process_priv.xnack_enabled !=
kfd_process_xnack_mode(p, true)) {
pr_err("xnack mode cannot be set\n");
ret = -EPERM;
goto exit;
}
}
pr_debug("set xnack mode: %d\n", process_priv.xnack_enabled);
p->xnack_enabled = process_priv.xnack_enabled;
+ pr_debug("set xnack mode:
%d\n", process_priv.xnack_mode);
+ p->xnack_enabled = process_priv.xnack_mode;
+ }
+
exit:
Re: [Intel-gfx] [PATCH 1/2] drm/dp: note that DPCD 0x2002-0x2003 match 0x200-0x201
On Mon, 10 Jan 2022, Ville Syrjälä wrote: > On Tue, Jan 04, 2022 at 08:48:56PM +0200, Jani Nikula wrote: >> DP_SINK_COUNT_ESI and DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0 have the same >> contents as DP_SINK_COUNT and DP_DEVICE_SERVICE_IRQ_VECTOR, >> respectively. > > IIRC there was an oversight in the earlier spec revisions that > showed bit 7 as reserved for one of the locations. But looks like > that got fixed. Yeah. Thanks for the review, pushed both to drm-misc-next. BR, Jani. > > Reviewed-by: Ville Syrjälä > >> >> Signed-off-by: Jani Nikula >> --- >> include/drm/drm_dp_helper.h | 7 ++- >> 1 file changed, 2 insertions(+), 5 deletions(-) >> >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> index 30359e434c3f..98d020835b49 100644 >> --- a/include/drm/drm_dp_helper.h >> +++ b/include/drm/drm_dp_helper.h >> @@ -1038,11 +1038,8 @@ struct drm_panel; >> #define DP_SIDEBAND_MSG_UP_REQ_BASE 0x1600 /* 1.2 MST */ >> >> /* DPRX Event Status Indicator */ >> -#define DP_SINK_COUNT_ESI 0x2002 /* 1.2 */ >> -/* 0-5 sink count */ >> -# define DP_SINK_COUNT_CP_READY (1 << 6) >> - >> -#define DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0 0x2003 /* 1.2 */ >> +#define DP_SINK_COUNT_ESI 0x2002 /* same as 0x200 */ >> +#define DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0 0x2003 /* same as 0x201 */ >> >> #define DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1 0x2004 /* 1.2 */ >> # define DP_RX_GTC_MSTR_REQ_STATUS_CHANGE(1 << 0) >> -- >> 2.30.2 -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v2 01/14] drm/edid: Don't clear YUV422 if using deep color
Hi Ville,
Thanks for your review
On Wed, Dec 15, 2021 at 03:48:39PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 15, 2021 at 01:43:53PM +0100, Maxime Ripard wrote:
> > The current code, when parsing the EDID Deep Color depths, that the
> > YUV422 cannot be used, referring to the HDMI 1.3 Specification.
> >
> > This specification, in its section 6.2.4, indeed states:
> >
> > For each supported Deep Color mode, RGB 4:4:4 shall be supported and
> > optionally YCBCR 4:4:4 may be supported.
> >
> > YCBCR 4:2:2 is not permitted for any Deep Color mode.
> >
> > This indeed can be interpreted like the code does, but the HDMI 1.4
> > specification further clarifies that statement in its section 6.2.4:
> >
> > For each supported Deep Color mode, RGB 4:4:4 shall be supported and
> > optionally YCBCR 4:4:4 may be supported.
> >
> > YCBCR 4:2:2 is also 36-bit mode but does not require the further use
> > of the Deep Color modes described in section 6.5.2 and 6.5.3.
> >
> > This means that, even though YUV422 can be used with 12 bit per color,
> > it shouldn't be treated as a deep color mode.
> >
> > This deviates from the interpretation of the code and comment, so let's
> > fix those.
> >
> > Fixes: d0c94692e0a3 ("drm/edid: Parse and handle HDMI deep color modes.")
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/gpu/drm/drm_edid.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 12893e7be89b..e57d1b8cdaaa 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -5106,10 +5106,9 @@ static void drm_parse_hdmi_deep_color_info(struct
> > drm_connector *connector,
> >
> > /*
> > * Deep color support mandates RGB444 support for all video
> > -* modes and forbids YCRCB422 support for all video modes per
> > -* HDMI 1.3 spec.
> > +* modes.
> > */
> > - info->color_formats = DRM_COLOR_FORMAT_RGB444;
> > + info->color_formats |= DRM_COLOR_FORMAT_RGB444;
> >
> > /* YCRCB444 is optional according to spec. */
> > if (hdmi[6] & DRM_EDID_HDMI_DC_Y444) {
>
> This whole code seems pretty much wrong. What it tries to do (I think)
> is make sure we don't use deep color with YCbCr 4:4:4 unless supported.
> But what it actually does is also disable YCbCr 4:4:4 8bpc when deep
> color is not supported for YCbCr 4:4:4.
>
> I think what we want is to just get rid of this color_formats stuff here
> entirely and instead have some kind of separate tracking of RGB 4:4:4 vs.
> YCbCr 4:4:4 deep color capabilities.
I'm sorry, I'm not entirely sure to understand what you're suggesting
here. Do you want to get ride of info->color_formats entirely?
Maxime
signature.asc
Description: PGP signature
Re: [Patch v4 23/24] drm/amdkfd: CRIU prepare for svm resume
On 2022-01-10 6:58 p.m., Felix Kuehling
wrote:
On
2022-01-05 9:43 a.m., philip yang wrote:
On 2021-12-22 7:37 p.m., Rajneesh Bhardwaj wrote:
During CRIU restore phase, the VMAs for
the virtual address ranges are
not at their final location yet so in this stage, only cache
the data
required to successfully resume the svm ranges during an
imminent CRIU
resume phase.
Signed-off-by: Rajneesh
Bhardwaj
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 4 +-
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 5 ++
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 99
drivers/gpu/drm/amd/amdkfd/kfd_svm.h | 12 +++
4 files changed, 118 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 916b8d000317..f7aa15b18f95 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2638,8 +2638,8 @@ static int criu_restore_objects(struct
file *filep,
goto exit;
break;
case KFD_CRIU_OBJECT_TYPE_SVM_RANGE:
- /* TODO: Implement SVM range */
- *priv_offset += sizeof(struct
kfd_criu_svm_range_priv_data);
+ ret = kfd_criu_restore_svm(p, (uint8_t __user
*)args->priv_data,
+ priv_offset,
max_priv_data_size);
if (ret)
goto exit;
break;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 87eb6739a78e..92191c541c29 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -790,6 +790,7 @@ struct svm_range_list {
struct list_head list;
struct work_struct deferred_list_work;
struct list_head deferred_range_list;
+ struct list_head criu_svm_metadata_list;
spinlock_t deferred_list_lock;
atomic_t evicted_ranges;
bool drain_pagefaults;
@@ -1148,6 +1149,10 @@ int kfd_criu_restore_event(struct file
*devkfd,
uint8_t __user *user_priv_data,
uint64_t *priv_data_offset,
uint64_t max_priv_data_size);
+int kfd_criu_restore_svm(struct kfd_process *p,
+ uint8_t __user *user_priv_data,
+ uint64_t *priv_data_offset,
+ uint64_t max_priv_data_size);
/* CRIU - End */
/* Queue Context Management */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 6d59f1bedcf2..e9f6c63c2a26 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -45,6 +45,14 @@
*/
#define AMDGPU_SVM_RANGE_RETRY_FAULT_PENDING 2000
+struct criu_svm_metadata {
+ struct list_head list;
+ __u64 start_addr;
+ __u64 size;
+ /* Variable length array of attributes */
+ struct kfd_ioctl_svm_attribute attrs[0];
+};
This data structure is struct kfd_criu_svm_range_priv_data plus
list_head, maybe you can add list_head to struct
kfd_criu_svm_range_priv_data and remove this new data structure,
then you can remove extra kzalloc, kfree for each svm object
resume and function svm_criu_prepare_for_resume could be
removed.
Adding list_head to the private structure is a bad idea, because
that structure is copied to/from user mode. Kernel mode po
[PATCH] drm/i915: Flip guc_id allocation partition
Move the multi-lrc guc_id from the lower allocation partition (0 to
number of multi-lrc guc_ids) to upper allocation partition (number of
single-lrc to max guc_ids).
Signed-off-by: Matthew Brost
---
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 57 ++-
1 file changed, 42 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 9989d121127df..1bacc9621cea8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -147,6 +147,8 @@ guc_create_parallel(struct intel_engine_cs **engines,
*/
#define NUMBER_MULTI_LRC_GUC_ID(guc) \
((guc)->submission_state.num_guc_ids / 16)
+#define NUMBER_SINGLE_LRC_GUC_ID(guc) \
+ ((guc)->submission_state.num_guc_ids - NUMBER_MULTI_LRC_GUC_ID(guc))
/*
* Below is a set of functions which control the GuC scheduling state which
@@ -1776,11 +1778,6 @@ int intel_guc_submission_init(struct intel_guc *guc)
INIT_WORK(&guc->submission_state.destroyed_worker,
destroyed_worker_func);
- guc->submission_state.guc_ids_bitmap =
- bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL);
- if (!guc->submission_state.guc_ids_bitmap)
- return -ENOMEM;
-
spin_lock_init(&guc->timestamp.lock);
INIT_DELAYED_WORK(&guc->timestamp.work, guc_timestamp_ping);
guc->timestamp.ping_delay = (POLL_TIME_CLKS / gt->clock_frequency + 1)
* HZ;
@@ -1796,7 +1793,8 @@ void intel_guc_submission_fini(struct intel_guc *guc)
guc_flush_destroyed_contexts(guc);
guc_lrc_desc_pool_destroy(guc);
i915_sched_engine_put(guc->sched_engine);
- bitmap_free(guc->submission_state.guc_ids_bitmap);
+ if (guc->submission_state.guc_ids_bitmap)
+ bitmap_free(guc->submission_state.guc_ids_bitmap);
}
static inline void queue_request(struct i915_sched_engine *sched_engine,
@@ -1863,6 +1861,33 @@ static void guc_submit_request(struct i915_request *rq)
spin_unlock_irqrestore(&sched_engine->lock, flags);
}
+static int new_mlrc_guc_id(struct intel_guc *guc, struct intel_context *ce)
+{
+ int ret;
+
+ GEM_BUG_ON(!intel_context_is_parent(ce));
+ GEM_BUG_ON(!guc->submission_state.guc_ids_bitmap);
+
+ ret = bitmap_find_free_region(guc->submission_state.guc_ids_bitmap,
+ NUMBER_MULTI_LRC_GUC_ID(guc),
+ order_base_2(ce->parallel.number_children
+ + 1));
+ if (likely(!(ret < 0)))
+ ret += NUMBER_SINGLE_LRC_GUC_ID(guc);
+
+ return ret;
+}
+
+static int new_slrc_guc_id(struct intel_guc *guc, struct intel_context *ce)
+{
+ GEM_BUG_ON(intel_context_is_parent(ce));
+
+ return ida_simple_get(&guc->submission_state.guc_ids,
+ 0, NUMBER_SINGLE_LRC_GUC_ID(guc),
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL |
+ __GFP_NOWARN);
+}
+
static int new_guc_id(struct intel_guc *guc, struct intel_context *ce)
{
int ret;
@@ -1870,16 +1895,10 @@ static int new_guc_id(struct intel_guc *guc, struct
intel_context *ce)
GEM_BUG_ON(intel_context_is_child(ce));
if (intel_context_is_parent(ce))
- ret =
bitmap_find_free_region(guc->submission_state.guc_ids_bitmap,
- NUMBER_MULTI_LRC_GUC_ID(guc),
-
order_base_2(ce->parallel.number_children
- + 1));
+ ret = new_mlrc_guc_id(guc, ce);
else
- ret = ida_simple_get(&guc->submission_state.guc_ids,
-NUMBER_MULTI_LRC_GUC_ID(guc),
-guc->submission_state.num_guc_ids,
-GFP_KERNEL | __GFP_RETRY_MAYFAIL |
-__GFP_NOWARN);
+ ret = new_slrc_guc_id(guc, ce);
+
if (unlikely(ret < 0))
return ret;
@@ -1989,6 +2008,14 @@ static int pin_guc_id(struct intel_guc *guc, struct
intel_context *ce)
GEM_BUG_ON(atomic_read(&ce->guc_id.ref));
+ if (unlikely(intel_context_is_parent(ce) &&
+!guc->submission_state.guc_ids_bitmap)) {
+ guc->submission_state.guc_ids_bitmap =
+ bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL);
+ if (!guc->submission_state.guc_ids_bitmap)
+ return -ENOMEM;
+ }
+
try_again:
spin_lock_irqsave(&guc->submission_state.lock, flags);
--
2.34.1
[PATCH] drm/i915: Lock timeline mutex directly in error path of eb_pin_timeline
Don't use the interruptable version of the timeline mutex lock in the
error path of eb_pin_timeline as the cleanup must always happen.
v2:
(John Harrison)
- Don't check for interrupt during mutex lock
v3:
(Tvrtko)
- A comment explaining why lock helper isn't used
Fixes: 544460c33821 ("drm/i915: Multi-BB execbuf")
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 9e221ce427075..4a611d62e991a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2518,9 +2518,14 @@ static int eb_pin_timeline(struct i915_execbuffer *eb,
struct intel_context *ce,
timeout) < 0) {
i915_request_put(rq);
- tl = intel_context_timeline_lock(ce);
+ /*
+* Error path, cannot use intel_context_timeline_lock as
+* that is user interruptable and this clean up step
+* must be done.
+*/
+ mutex_lock(&ce->timeline->mutex);
intel_context_exit(ce);
- intel_context_timeline_unlock(tl);
+ mutex_unlock(&ce->timeline->mutex);
if (nonblock)
return -EWOULDBLOCK;
--
2.34.1
[PATCH 0/4] dicsrete card 64K page support
This series continues support for 64K pages for discrete cards. It supersedes the 64K patches from https://patchwork.freedesktop.org/series/95686/#rev4 Changes since that series: - set min alignment for DG2 to 2MB in i915_address_space_init - replace coloring with simpler 2MB VA alignment for lmem buffers - enforce alignment to 2MB for lmem objects on DG2 in i915_vma_insert - expand vma reservation to round up to 2MB on DG2 in i915_vma_insert - add alignment test Matthew Auld (3): drm/i915: enforce min GTT alignment for discrete cards drm/i915: support 64K GTT pages for discrete cards drm/i915/uapi: document behaviour for DG2 64K support Robert Beckett (1): drm/i915: add gtt misalignment test .../gpu/drm/i915/gem/selftests/huge_pages.c | 60 + .../i915/gem/selftests/i915_gem_client_blt.c | 23 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 109 - drivers/gpu/drm/i915/gt/intel_gtt.c | 14 ++ drivers/gpu/drm/i915/gt/intel_gtt.h | 12 + drivers/gpu/drm/i915/gt/intel_ppgtt.c | 1 + drivers/gpu/drm/i915/i915_vma.c | 14 ++ drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 226 +++--- include/uapi/drm/i915_drm.h | 44 +++- 9 files changed, 454 insertions(+), 49 deletions(-) -- 2.25.1
[PATCH 1/4] drm/i915: enforce min GTT alignment for discrete cards
From: Matthew Auld
For local-memory objects we need to align the GTT addresses
to 64K, both for the ppgtt and ggtt.
We need to support vm->min_alignment > 4K, depending
on the vm itself and the type of object we are inserting.
With this in mind update the GTT selftests to take this
into account.
For DG2 we further align and pad lmem object GTT addresses
to 2MB to ensure PDEs contain consistent page sizes as
required by the HW.
Signed-off-by: Matthew Auld
Signed-off-by: Ramalingam C
Signed-off-by: Robert Beckett
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
---
.../i915/gem/selftests/i915_gem_client_blt.c | 23 +++--
drivers/gpu/drm/i915/gt/intel_gtt.c | 14 +++
drivers/gpu/drm/i915/gt/intel_gtt.h | 9 ++
drivers/gpu/drm/i915/i915_vma.c | 14 +++
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 96 ---
5 files changed, 115 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
index c08f766e6e15..7fee95a65414 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
@@ -39,6 +39,7 @@ struct tiled_blits {
struct blit_buffer scratch;
struct i915_vma *batch;
u64 hole;
+ u64 align;
u32 width;
u32 height;
};
@@ -410,14 +411,21 @@ tiled_blits_create(struct intel_engine_cs *engine, struct
rnd_state *prng)
goto err_free;
}
- hole_size = 2 * PAGE_ALIGN(WIDTH * HEIGHT * 4);
+ t->align = I915_GTT_PAGE_SIZE_2M; /* XXX worst case, derive from vm! */
+ t->align = max(t->align,
+ i915_vm_min_alignment(t->ce->vm, INTEL_MEMORY_LOCAL));
+ t->align = max(t->align,
+ i915_vm_min_alignment(t->ce->vm, INTEL_MEMORY_SYSTEM));
+
+ hole_size = 2 * round_up(WIDTH * HEIGHT * 4, t->align);
hole_size *= 2; /* room to maneuver */
- hole_size += 2 * I915_GTT_MIN_ALIGNMENT;
+ hole_size += 2 * t->align; /* padding on either side */
mutex_lock(&t->ce->vm->mutex);
memset(&hole, 0, sizeof(hole));
err = drm_mm_insert_node_in_range(&t->ce->vm->mm, &hole,
- hole_size, 0, I915_COLOR_UNEVICTABLE,
+ hole_size, t->align,
+ I915_COLOR_UNEVICTABLE,
0, U64_MAX,
DRM_MM_INSERT_BEST);
if (!err)
@@ -428,7 +436,7 @@ tiled_blits_create(struct intel_engine_cs *engine, struct
rnd_state *prng)
goto err_put;
}
- t->hole = hole.start + I915_GTT_MIN_ALIGNMENT;
+ t->hole = hole.start + t->align;
pr_info("Using hole at %llx\n", t->hole);
err = tiled_blits_create_buffers(t, WIDTH, HEIGHT, prng);
@@ -455,7 +463,7 @@ static void tiled_blits_destroy(struct tiled_blits *t)
static int tiled_blits_prepare(struct tiled_blits *t,
struct rnd_state *prng)
{
- u64 offset = PAGE_ALIGN(t->width * t->height * 4);
+ u64 offset = round_up(t->width * t->height * 4, t->align);
u32 *map;
int err;
int i;
@@ -486,8 +494,7 @@ static int tiled_blits_prepare(struct tiled_blits *t,
static int tiled_blits_bounce(struct tiled_blits *t, struct rnd_state *prng)
{
- u64 offset =
- round_up(t->width * t->height * 4, 2 * I915_GTT_MIN_ALIGNMENT);
+ u64 offset = round_up(t->width * t->height * 4, 2 * t->align);
int err;
/* We want to check position invariant tiling across GTT eviction */
@@ -500,7 +507,7 @@ static int tiled_blits_bounce(struct tiled_blits *t, struct
rnd_state *prng)
/* Reposition so that we overlap the old addresses, and slightly off */
err = tiled_blit(t,
-&t->buffers[2], t->hole + I915_GTT_MIN_ALIGNMENT,
+&t->buffers[2], t->hole + t->align,
&t->buffers[1], t->hole + 3 * offset / 2);
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index a94be0306464..156852dcf33a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -219,6 +219,20 @@ void i915_address_space_init(struct i915_address_space
*vm, int subclass)
GEM_BUG_ON(!vm->total);
drm_mm_init(&vm->mm, 0, vm->total);
+
+ memset64(vm->min_alignment, I915_GTT_MIN_ALIGNMENT,
+ARRAY_SIZE(vm->min_alignment));
+
+ if (HAS_64K_PAGES(vm->i915)) {
+ if (IS_DG2(vm->i915)) {
+ vm->min_alignment[INTEL_MEMORY_LOCAL] =
I915_GTT_PAGE_SIZE_2M;
+ vm->min_alignment[INTEL_MEMORY_STOLEN_LOCAL] =
I915_GTT_PAGE_SIZE_2M;
+ } else
[PATCH 2/4] drm/i915: support 64K GTT pages for discrete cards
From: Matthew Auld
discrete cards optimise 64K GTT pages for local-memory, since everything
should be allocated at 64K granularity. We say goodbye to sparse
entries, and instead get a compact 256B page-table for 64K pages,
which should be more cache friendly. 4K pages for local-memory
are no longer supported by the HW.
Signed-off-by: Matthew Auld
Signed-off-by: Stuart Summers
Signed-off-by: Ramalingam C
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
---
.../gpu/drm/i915/gem/selftests/huge_pages.c | 60 ++
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 109 +-
drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 1 +
4 files changed, 170 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 11f0aa65f8a3..ef3439b290ca 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1483,6 +1483,65 @@ static int igt_ppgtt_sanity_check(void *arg)
return err;
}
+static int igt_ppgtt_compact(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj;
+ int err;
+
+ /*
+* Simple test to catch issues with compact 64K pages -- since the pt is
+* compacted to 256B that gives us 32 entries per pt, however since the
+* backing page for the pt is 4K, any extra entries we might incorrectly
+* write out should be ignored by the HW. If ever hit such a case this
+* test should catch it since some of our writes would land in scratch.
+*/
+
+ if (!HAS_64K_PAGES(i915)) {
+ pr_info("device lacks compact 64K page support, skipping\n");
+ return 0;
+ }
+
+ if (!HAS_LMEM(i915)) {
+ pr_info("device lacks LMEM support, skipping\n");
+ return 0;
+ }
+
+ /* We want the range to cover multiple page-table boundaries. */
+ obj = i915_gem_object_create_lmem(i915, SZ_4M, 0);
+ if (IS_ERR(obj))
+ return err;
+
+ err = i915_gem_object_pin_pages_unlocked(obj);
+ if (err)
+ goto out_put;
+
+ if (obj->mm.page_sizes.phys < I915_GTT_PAGE_SIZE_64K) {
+ pr_info("LMEM compact unable to allocate huge-page(s)\n");
+ goto out_unpin;
+ }
+
+ /*
+* Disable 2M GTT pages by forcing the page-size to 64K for the GTT
+* insertion.
+*/
+ obj->mm.page_sizes.sg = I915_GTT_PAGE_SIZE_64K;
+
+ err = igt_write_huge(i915, obj);
+ if (err)
+ pr_err("LMEM compact write-huge failed\n");
+
+out_unpin:
+ i915_gem_object_unpin_pages(obj);
+out_put:
+ i915_gem_object_put(obj);
+
+ if (err == -ENOMEM)
+ err = 0;
+
+ return err;
+}
+
static int igt_tmpfs_fallback(void *arg)
{
struct drm_i915_private *i915 = arg;
@@ -1740,6 +1799,7 @@ int i915_gem_huge_page_live_selftests(struct
drm_i915_private *i915)
SUBTEST(igt_tmpfs_fallback),
SUBTEST(igt_ppgtt_smoke_huge),
SUBTEST(igt_ppgtt_sanity_check),
+ SUBTEST(igt_ppgtt_compact),
};
if (!HAS_PPGTT(i915)) {
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index b012c50f7ce7..8d081497e87e 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -233,6 +233,8 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space *
const vm,
start, end, lvl);
} else {
unsigned int count;
+ unsigned int pte = gen8_pd_index(start, 0);
+ unsigned int num_ptes;
u64 *vaddr;
count = gen8_pt_count(start, end);
@@ -242,10 +244,18 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space *
const vm,
atomic_read(&pt->used));
GEM_BUG_ON(!count || count >= atomic_read(&pt->used));
+ num_ptes = count;
+ if (pt->is_compact) {
+ GEM_BUG_ON(num_ptes % 16);
+ GEM_BUG_ON(pte % 16);
+ num_ptes /= 16;
+ pte /= 16;
+ }
+
vaddr = px_vaddr(pt);
- memset64(vaddr + gen8_pd_index(start, 0),
+ memset64(vaddr + pte,
vm->scratch[0]->encode,
-count);
+num_ptes);
atomic_sub(count, &pt->used);
start += count;
@@ -453,6 +463,96 @@ gen8_ppgtt_insert_pte(struct i915_ppgtt *ppgtt,
[PATCH 3/4] drm/i915: add gtt misalignment test
add test to check handling of misaligned offsets and sizes
Signed-off-by: Robert Beckett
---
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 130 ++
1 file changed, 130 insertions(+)
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index fea031b4ec4f..28de0b333835 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -22,10 +22,12 @@
*
*/
+#include "gt/intel_gtt.h"
#include
#include
#include "gem/i915_gem_context.h"
+#include "gem/i915_gem_region.h"
#include "gem/selftests/mock_context.h"
#include "gt/intel_context.h"
#include "gt/intel_gpu_commands.h"
@@ -1066,6 +1068,120 @@ static int shrink_boom(struct i915_address_space *vm,
return err;
}
+static int misaligned_case(struct i915_address_space *vm, struct
intel_memory_region *mr,
+ u64 addr, u64 size, unsigned long flags)
+{
+ struct drm_i915_gem_object *obj;
+ struct i915_vma *vma;
+ int err = 0;
+ u64 expected_vma_size, expected_node_size;
+
+ obj = i915_gem_object_create_region(mr, size, 0, 0);
+ if (IS_ERR(obj))
+ return PTR_ERR(obj);
+
+ vma = i915_vma_instance(obj, vm, NULL);
+ if (IS_ERR(vma)) {
+ err = PTR_ERR(vma);
+ goto err_put;
+ }
+
+ err = i915_vma_pin(vma, 0, 0, addr | flags);
+ if (err)
+ goto err_put;
+ i915_vma_unpin(vma);
+
+ if (!drm_mm_node_allocated(&vma->node)) {
+ err = -EINVAL;
+ goto err_put;
+ }
+
+ if (i915_vma_misplaced(vma, 0, 0, addr | flags)) {
+ err = -EINVAL;
+ goto err_put;
+ }
+
+ expected_vma_size = round_up(size, 1 << (ffs(vma->page_sizes.gtt) - 1));
+ expected_node_size = expected_vma_size;
+
+ if (IS_DG2(vm->i915) && i915_gem_object_is_lmem(obj)) {
+ /* dg2 should expand lmem node to 2MB */
+ expected_vma_size = round_up(size, I915_GTT_PAGE_SIZE_64K);
+ expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_2M);
+ }
+
+ if (vma->size != expected_vma_size || vma->node.size !=
expected_node_size) {
+ err = i915_vma_unbind(vma);
+ err = -EBADSLT;
+ goto err_put;
+ }
+
+ err = i915_vma_unbind(vma);
+ if (err)
+ goto err_put;
+
+ GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
+
+err_put:
+ i915_gem_object_put(obj);
+ cleanup_freed_objects(vm->i915);
+ return err;
+}
+
+static int misaligned_pin(struct i915_address_space *vm,
+ u64 hole_start, u64 hole_end,
+ unsigned long end_time)
+{
+ struct intel_memory_region *mr;
+ enum intel_region_id id;
+ unsigned long flags = PIN_OFFSET_FIXED | PIN_USER;
+ int err = 0;
+ u64 hole_size = hole_end - hole_start;
+
+ if (i915_is_ggtt(vm))
+ flags |= PIN_GLOBAL;
+
+ for_each_memory_region(mr, vm->i915, id) {
+ u64 min_alignment = i915_vm_min_alignment(vm, id);
+ u64 size = min_alignment;
+ u64 addr = round_up(hole_start + (hole_size / 2),
min_alignment);
+
+ /* we can't test < 4k alignment due to flags being encoded in
lower bits */
+ if (min_alignment != I915_GTT_PAGE_SIZE_4K) {
+ err = misaligned_case(vm, mr, addr + (min_alignment /
2), size, flags);
+ /* misaligned should error with -EINVAL*/
+ if (!err)
+ err = -EBADSLT;
+ if (err != -EINVAL)
+ return err;
+ }
+
+ /* test for vma->size expansion to min page size */
+ err = misaligned_case(vm, mr, addr, PAGE_SIZE, flags);
+ if (min_alignment > hole_size) {
+ if (!err)
+ err = -EBADSLT;
+ else if (err == -ENOSPC)
+ err = 0;
+ }
+ if (err)
+ return err;
+
+ /* test for intermediate size not expanding vma->size for large
alignments */
+ err = misaligned_case(vm, mr, addr, size / 2, flags);
+ if (min_alignment > hole_size) {
+ if (!err)
+ err = -EBADSLT;
+ else if (err == -ENOSPC)
+ err = 0;
+ }
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
static int exercise_ppgtt(struct drm_i915_private *dev_priv,
int (*func)(struct i915_address_space *vm,
u64 hole_start, u64 hole_end,
@@ -11
[PATCH 4/4] drm/i915/uapi: document behaviour for DG2 64K support
From: Matthew Auld
On discrete platforms like DG2, we need to support a minimum page size
of 64K when dealing with device local-memory. This is quite tricky for
various reasons, so try to document the new implicit uapi for this.
v2: Fixed suggestions on formatting [Daniel]
Signed-off-by: Matthew Auld
Signed-off-by: Ramalingam C
Signed-off-by: Robert Beckett
cc: Simon Ser
cc: Pekka Paalanen
Cc: Jordan Justen
Cc: Kenneth Graunke
Cc: mesa-...@lists.freedesktop.org
Cc: Tony Ye
Cc: Slawomir Milczarek
---
include/uapi/drm/i915_drm.h | 44 -
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5e678917da70..486b7b96291e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1118,10 +1118,16 @@ struct drm_i915_gem_exec_object2 {
/**
* When the EXEC_OBJECT_PINNED flag is specified this is populated by
* the user with the GTT offset at which this object will be pinned.
+*
* When the I915_EXEC_NO_RELOC flag is specified this must contain the
* presumed_offset of the object.
+*
* During execbuffer2 the kernel populates it with the value of the
* current GTT offset of the object, for future presumed_offset writes.
+*
+* See struct drm_i915_gem_create_ext for the rules when dealing with
+* alignment restrictions with I915_MEMORY_CLASS_DEVICE, on devices with
+* minimum page sizes, like DG2.
*/
__u64 offset;
@@ -3145,11 +3151,39 @@ struct drm_i915_gem_create_ext {
*
* The (page-aligned) allocated size for the object will be returned.
*
-* Note that for some devices we have might have further minimum
-* page-size restrictions(larger than 4K), like for device local-memory.
-* However in general the final size here should always reflect any
-* rounding up, if for example using the
I915_GEM_CREATE_EXT_MEMORY_REGIONS
-* extension to place the object in device local-memory.
+*
+* **DG2 64K min page size implications:**
+*
+* On discrete platforms, starting from DG2, we have to contend with GTT
+* page size restrictions when dealing with I915_MEMORY_CLASS_DEVICE
+* objects. Specifically the hardware only supports 64K or larger GTT
+* page sizes for such memory. The kernel will already ensure that all
+* I915_MEMORY_CLASS_DEVICE memory is allocated using 64K or larger page
+* sizes underneath.
+*
+* Note that the returned size here will always reflect any required
+* rounding up done by the kernel, i.e 4K will now become 64K on devices
+* such as DG2.
+*
+* **Special DG2 GTT address alignment requirement:**
+*
+* The GTT alignment will also need be at least 2M for such objects.
+*
+* Note that due to how the hardware implements 64K GTT page support, we
+* have some further complications:
+*
+* 1) The entire PDE(which covers a 2MB virtual address range), must
+* contain only 64K PTEs, i.e mixing 4K and 64K PTEs in the same
+* PDE is forbidden by the hardware.
+*
+* 2) We still need to support 4K PTEs for I915_MEMORY_CLASS_SYSTEM
+* objects.
+*
+* To keep things simple for userland, we mandate that any GTT mappings
+* must be aligned to and rounded up to 2MB. As this only wastes virtual
+* address space and avoids userland having to copy any needlessly
+* complicated PDE sharing scheme (coloring) and only affects GD2, this
+* id deemed to be a good compromise.
*/
__u64 size;
/**
--
2.25.1
