Re: [PATCH v2] drm/prime: fix extracting of the DMA addresses from a scatterlist
Hi Michael,
On 2020-03-27 19:31, Ruhl, Michael J wrote:
>> -Original Message-
>> From: Marek Szyprowski
>> Sent: Friday, March 27, 2020 12:21 PM
>> To: dri-devel@lists.freedesktop.org; linux-samsung-...@vger.kernel.org;
>> linux-ker...@vger.kernel.org
>> Cc: Marek Szyprowski ;
>> sta...@vger.kernel.org; Bartlomiej Zolnierkiewicz
>> ; Maarten Lankhorst
>> ; Maxime Ripard
>> ; Thomas Zimmermann ;
>> David Airlie ; Daniel Vetter ; Alex
>> Deucher
>> ; Shane Francis ;
>> Ruhl, Michael J
>> Subject: [PATCH v2] drm/prime: fix extracting of the DMA addresses from a
>> scatterlist
>>
>> Scatterlist elements contains both pages and DMA addresses, but one
>> should not assume 1:1 relation between them. The sg->length is the size
>> of the physical memory chunk described by the sg->page, while
>> sg_dma_len(sg) is the size of the DMA (IO virtual) chunk described by
>> the sg_dma_address(sg).
>>
>> The proper way of extracting both: pages and DMA addresses of the whole
>> buffer described by a scatterlist it to iterate independently over the
>> sg->pages/sg->length and sg_dma_address(sg)/sg_dma_len(sg) entries.
>>
>> Fixes: 42e67b479eab ("drm/prime: use dma length macro when mapping sg")
>> Signed-off-by: Marek Szyprowski
>> Reviewed-by: Alex Deucher
>> ---
>> drivers/gpu/drm/drm_prime.c | 37 +---
>> -
>> 1 file changed, 25 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>> index 1de2cde2277c..282774e469ac 100644
>> --- a/drivers/gpu/drm/drm_prime.c
>> +++ b/drivers/gpu/drm/drm_prime.c
>> @@ -962,27 +962,40 @@ int drm_prime_sg_to_page_addr_arrays(struct
>> sg_table *sgt, struct page **pages,
>> unsigned count;
>> struct scatterlist *sg;
>> struct page *page;
>> -u32 len, index;
>> +u32 page_len, page_index;
>> dma_addr_t addr;
>> +u32 dma_len, dma_index;
>>
>> -index = 0;
>> +/*
>> + * Scatterlist elements contains both pages and DMA addresses, but
>> + * one shoud not assume 1:1 relation between them. The sg->length
>> is
>> + * the size of the physical memory chunk described by the sg->page,
>> + * while sg_dma_len(sg) is the size of the DMA (IO virtual) chunk
>> + * described by the sg_dma_address(sg).
>> + */
> Is there an example of what the scatterlist would look like in this case?
DMA framework or IOMMU is allowed to join consecutive chunks while
mapping if such operation is supported by the hw. Here is the example:
Lets assume that we have a scatterlist with 4 4KiB pages of the physical
addresses: 0x1200, 0x13011000, 0x13012000, 0x11011000. The total
size of the buffer is 16KiB. After mapping this scatterlist to a device
behind an IOMMU it may end up as a contiguous buffer in the DMA (IOVA)
address space. at 0xf001. The scatterlist will look like this:
sg[0].page = 0x1200
sg[0].len = 4096
sg[0].dma_addr = 0xf001
sg[0].dma_len = 16384
sg[1].page = 0x13011000
sg[1].len = 4096
sg[1].dma_addr = 0
sg[1].dma_len = 0
sg[2].page = 0x13012000
sg[2].len = 4096
sg[2].dma_addr = 0
sg[2].dma_len = 0
sg[3].page = 0x11011000
sg[3].len = 4096
sg[3].dma_addr = 0
sg[3].dma_len = 0
(I've intentionally wrote page as physical address to make it easier to
understand, in real SGs it is stored a struct page pointer).
> Does each SG entry always have the page and dma info? or could you have
> entries that have page information only, and entries that have dma info only?
When SG is not mapped yet it contains only the ->pages and ->len
entries. I'm not aware of the SGs with the DMA information only, but in
theory it might be possible to have such.
> If the same entry has different size info (page_len = PAGE_SIZE,
> dma_len = 4 * PAGE_SIZE?), are we guaranteed that the arrays (page and addrs)
> have
> been sized correctly?
There are always no more DMA related entries than the phys pages. If
there is 1:1 mapping between physical memory and DMA (IOVA) space, then
each SG entry will have len == dma_len, and dma_addr will be describing
the same as page entry. DMA mapping framework is allowed only to join
entries while mapping to DMA (IOVA).
> Just trying to get my head wrapped around this.
Sure, I hope my explanation helps a bit.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 2/2] drm/lima: Add optional devfreq and cooling device support
I'm not the maintainer of patch 1 file, so please contact: - Rob Herring - Maxime Ripard - Heiko Stuebner to review and apply patch 1. Regards, Qiang On Sat, Mar 28, 2020 at 6:20 PM Martin Blumenstingl wrote: > > On Sat, Mar 28, 2020 at 9:40 AM Qiang Yu wrote: > > > > Applied to drm-misc-next. > thank you! > > regarding patch #1 - can you apply this as well? > patch #1 just takes this midgard change [0] and ports it to utgard > > > Thank you! > Martin > > > [0] > https://cgit.freedesktop.org/drm/drm-misc/commit/Documentation/devicetree/bindings/gpu?id=982c0500fd1a8012c31d3c9dd8de285129904656 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Outreachy kernel] [PATCH] staging: fbtft: Replace udelay with preferred usleep_range
On Sun, Mar 29, 2020 at 2:23 PM Sam Muhammed wrote: > On Sun, 2020-03-29 at 12:37 +0200, Julia Lawall wrote: > > On Sun, 29 Mar 2020, Soumyajit Deb wrote: > > First of all, let's stop topposting. > > > I had the same doubt the other day about the replacement of udelay() with > > > usleep_range(). The corresponding range for the single argument value of > > > udelay() is quite confusing as I couldn't decide the range. But as much > > > as I > > > noticed checkpatch.pl gives warning for replacing udelay() with > > > usleep_range() by checking the argument value of udelay(). In the > > > documentation, it is written udelay() should be used for a sleep time of > > > at > > > most 10 microseconds but between 10 microseconds and 20 milliseconds, > > > usleep_range() should be used. > > > I think the range is code specific and will depend on what range is > > > acceptable and doesn't break the code. > > > Please correct me if I am wrong. > > > > The range depends on the associated hardware. Just because checkpatch > > suggests something doesn't mean that it is easy to address the problem. > Hi all, i think when it comes to a significant change in the code, we > should at least be familiar with the driver or be able to test the > change. > > In the very beginning of the Documentation/timers/timers-howto.rst > there is the question: > "Is my code in an atomic context?" > It's not just about the range, it's more of at which context this code > runs, for atomic-context -> udelay must be used. > for non-atomic context -> usleep-range is better for power-management. > > unless we are familiar with the driver we wouldn't really know in what > context this code is run at. > > This thread though had the same conversation about this change, for the > same driver. > https://patchwork.kernel.org/patch/11137125/ While it's a good discussion it reminds me that this entire function, i.e. reset(), repeats the on provided by fbtft core. Yes, the only question if it's atomic or not. IIRC ->reset() is being called only in non-atomic contexts and keeping reset signal longer is fine (but better to check with datasheet). So, I would rather to drop the function completely in order to use fbtft's core one. -- With Best Regards, Andy Shevchenko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: add docs about the IN_FORMATS plane property
This is a standard property attached to planes in drm_universal_plane_init when drm_mode_config.allow_fb_modifiers is true. Signed-off-by: Simon Ser Cc: Daniel Vetter Cc: Daniel Stone --- drivers/gpu/drm/drm_blend.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index 121481f6aa71..88eedee018d3 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -183,6 +183,12 @@ * plane does not expose the "alpha" property, then this is * assumed to be 1.0 * + * IN_FORMATS: + * Blob property which contains the set of buffer format and modifier + * pairs supported by this plane. The blob is a drm_format_modifier_blob + * struct. Without this property the plane doesn't support buffers with + * modifiers. Userspace cannot change this property. + * * Note that all the property extensions described here apply either to the * plane or the CRTC (e.g. for the background color, which currently is not * exposed and assumed to be black). -- 2.26.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 0/2] Add support for rm69299 Visionox panel driver and add devicetree bindings for visionox panel
Hi Sam, On Sat, Mar 28, 2020 at 09:40:47PM +0100, Sam Ravnborg wrote: > Hi Harigovindan > > On Fri, Mar 27, 2020 at 01:06:34PM +0530, Harigovindan P wrote: > > Adding support for visionox rm69299 panel driver and adding bindings for > > the same panel. > > > > Harigovindan P (2): > > dt-bindings: display: add visionox rm69299 panel variant > > drm/panel: add support for rm69299 visionox panel driver > > I have only the first patch, which is now applied. > Please resend second patch as it is lost somewhere. Yes, it seems for v8, v9 and v10 only the bindings were sent, even though the cover letter and subject say it's a series of two patches. To my knowledge the latest version of the driver patch is this: https://patchwork.kernel.org/patch/11439689/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 32/36] dt-bindings: display: convert sharp, ls037v7dw01 to DT Schema
Hi Rob. > > + > > + mode-gpios: > > +description: | > > + GPIO ordered MO, LR, and UD as specified in LS037V7DW01.pdf > > 3 or... > > > + change configuration between QVGA and VGA mode and the > > + scan direction. As these pins can be also configured > > + with external pulls, all the GPIOs are considered > > + optional with holes in the array. > > minItems: 3 > maxItems: 5 This binding can specify up to three GPIOs like this: > > +mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH/* gpio154, lcd MO > > */ > > + &gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */ > > + &gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */ They are in the linux kernel driver accessed like this: devm_gpiod_get_index(&pdev->dev, "mode", 2, GPIOD_OUT_LOW); The following is OK in the DT file: mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>; mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH &gpio1 2 GPIO_ACTIVE_HIGH>; mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH &gpio1 2 GPIO_ACTIVE_HIGH &gpio1 3 GPIO_ACTIVE_HIGH>; But the following is not OK: mode-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>, <&gpio1 2 GPIO_ACTIVE_HIGH>; Any hints how to specify the binding to prevent the above? I have tried a few combinations - but they do not catch this. So my binding attempts are not restrictive enough. Any hints how to describe this properly? Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] meson.build: Don't detect header for linux
On Wednesday, 2020-01-29 09:53:16 +, Eric Engestrom wrote:
> On Friday, 2020-01-10 13:30:41 +0900, Seung-Woo Kim wrote:
> > The header is not required for Linux and GNU libc
> > 2.30 starts to warn about Linux specific header
> > deprecation. Don't detect header for linux.
> >
> > Signed-off-by: Seung-Woo Kim
> > ---
> > Fix meson.build script instead of code itself as commented below:
> > https://patchwork.kernel.org/patch/11325345/
> > ---
> > meson.build | 15 +++
> > 1 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 782b1a3..b1c557a 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -183,10 +183,17 @@ else
> >dep_rt = []
> > endif
> > dep_m = cc.find_library('m', required : false)
> > -# From Niclas Zeising:
> > -# FreeBSD requires sys/types.h for sys/sysctl.h, add it as part of the
> > -# includes when checking for headers.
> > -foreach header : ['sys/sysctl.h', 'sys/select.h', 'alloca.h']
> > +if not ['linux'].contains(host_machine.system())
> > + # From Niclas Zeising:
> > + # FreeBSD requires sys/types.h for sys/sysctl.h, add it as part of the
> > + # includes when checking for headers.
> > + foreach header : ['sys/sysctl.h']
> > +config.set('HAVE_' + header.underscorify().to_upper(),
> > + cc.compiles('#include \n#include <@0@>'.format(header),
> > name : '@0@ works'.format(header)))
> > + endforeach
> > +endif
> > +endforeach
>
> Stray `endforeach`.
>
> Could you post your patch as a Merge Request [1] instead of on the mailing
> list?
> The automatic testing there means it would instantly catch mistakes like
> these :)
>
> [1] https://gitlab.freedesktop.org/mesa/drm/merge_requests
>
> > +foreach header : ['sys/select.h', 'alloca.h']
> >config.set('HAVE_' + header.underscorify().to_upper(),
> > cc.compiles('#include \n#include <@0@>'.format(header),
> > name : '@0@ works'.format(header)))
>
> Can you drop the `#include \n` now that sys/sysctl.h is
> being split out?
>
> Note that since https://gitlab.freedesktop.org/mesa/drm/merge_requests/8
> we now use config.set10(), which means you'll need to refactor a tiny
> bit (move the !linux condition inside the config.set10() call).
>
> The new code block should look like this:
>
> # From Niclas Zeising:
> # FreeBSD requires sys/types.h for sys/sysctl.h, add it as part of the
> # includes when checking for headers.
> foreach header : ['sys/sysctl.h']
> config.set10('HAVE_' + header.underscorify().to_upper(),
>not ['linux'].contains(host_machine.system()) and
>cc.compiles('#include \n#include <@0@>'.format(header),
> name : '@0@ works'.format(header)))
> endforeach
FYI, I have posted a variant of the above as a merge request:
https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/53
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/bridge: anx6345: set correct BPC for display_info of connector
Some drivers (e.g. sun4i-drm) need this info to decide whether they
need to enable dithering. Currently driver reports what panel supports
and if panel supports 8 we don't get dithering enabled.
Hardcode BPC to 6 for now since that's the only BPC
that driver supports.
Fixes: 6aa192698089 ("drm/bridge: Add Analogix anx6345 support")
Signed-off-by: Vasily Khoruzhick
---
drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
index d7cb10c599a3..ea5de9395662 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
@@ -494,6 +494,9 @@ static int anx6345_get_modes(struct drm_connector
*connector)
num_modes += drm_add_edid_modes(connector, anx6345->edid);
+ /* Driver currently supports only 6bpc */
+ connector->display_info.bpc = 6;
+
unlock:
if (power_off)
anx6345_poweroff(anx6345);
--
2.25.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
