Re: [PATCH 5/6] drm/mediatek: Convert to use CMA helpers
Hi, Rob: On Wed, 2019-10-23 at 17:56 -0500, Rob Herring wrote: > On Wed, Oct 23, 2019 at 4:06 PM CK Hu wrote: > > > > Hi, Rob: > > > > On Wed, 2019-10-23 at 12:42 -0500, Rob Herring wrote: > > > On Tue, Oct 22, 2019 at 12:07 PM Matthias Brugger > > > wrote: > > > > > > > > Hi Rob, > > > > > > > > On 21/10/2019 23:45, Rob Herring wrote: > > > > > The only reason the Mediatek driver doesn't use the CMA helpers is it > > > > > sets DMA_ATTR_NO_KERNEL_MAPPING and does a vmap() on demand. Using > > > > > vmap() is not even guaranteed to work as DMA buffers may not have a > > > > > struct page. Now that the CMA helpers support setting > > > > > DMA_ATTR_NO_KERNEL_MAPPING as needed or not, convert Mediatek driver > > > > > to > > > > > use CMA helpers. > > > > > > > > > > Cc: CK Hu > > > > > Cc: Philipp Zabel > > > > > Cc: David Airlie > > > > > Cc: Daniel Vetter > > > > > Cc: Matthias Brugger > > > > > Cc: linux-arm-ker...@lists.infradead.org > > > > > Cc: linux-media...@lists.infradead.org > > > > > Signed-off-by: Rob Herring > > > > > --- > > > > > > > > I tested this on my Chromebook with some patches on top of v5.4-rc1 > > > > [1], which > > > > work. If I add your patches on top of that, the system does not boot up. > > > > Unfortunately I don't have a serial console, so I wasn't able to see if > > > > there is > > > > any error message. > > > > > > Thanks for testing. I'm based on drm-misc-next, but don't see anything > > > obvious there that would matter. There are some mmap changes, but I > > > think they shouldn't matter. > > > > > > Did you have fbcon enabled? That may give more clues about where the > > > problem is. > > > > There are priv->dma_dev for dma device, but it is not drm device. In > > mt8173.dtsi [1], there are mmsys device and ovl device, mmsys device is > > drm device and ovl device is mmsys's sub device which provide dma > > function, so ovl is the priv->dma_dev. I think your patch directly use > > drm device for dma operation and this would cause dma function fail. > > Please use priv->dma_dev for dma operation. > > Right, thanks for catching that. Either we'll need to make CMA GEM > object have a struct device ptr or adjust the drm_device.dev to have > the necessary DMA setup. > > One question though, why do you use CMA when you have an IOMMU? That's > not optimal as CMA size may be limited. Or you don't always have an > IOMMU? For all upstreamed mediatek SoC, all has IOMMU, so it does not need CMA. I think we use CMA just because we refer to other drm driver to implement mediatek drm driver and we misused CMA helper function but it works. I think we should change to more accurate implementation. If you want, you could modify it in this series. Regards, CK > > Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 0/9] backlight: gpio: simplify the driver
Hello,
On Thu, Oct 24, 2019 at 07:47:26AM +0100, Lee Jones wrote:
> On Wed, 23 Oct 2019, Daniel Thompson wrote:
>
> > On Tue, Oct 22, 2019 at 11:29:54AM +0200, Bartosz Golaszewski wrote:
> > > wt., 22 paź 2019 o 10:36 Bartosz Golaszewski napisał(a):
> > > >
> > > > From: Bartosz Golaszewski
> > > >
> > > > While working on my other series related to gpio-backlight[1] I noticed
> > > > that we could simplify the driver if we made the only user of platform
> > > > data use GPIO lookups and device properties. This series tries to do
> > > > that.
> > > >
> > > > First two patches contain minor fixes. Third patch makes the driver
> > > > explicitly drive the GPIO line. Fourth patch adds all necessary data
> > > > structures to ecovec24. Patch 5/9 unifies much of the code for both
> > > > pdata and non-pdata cases. Patches 6-7/9 remove unused platform data
> > > > fields. Last two patches contain additional improvements for the GPIO
> > > > backlight driver while we're already modifying it.
> > > >
> > > > I don't have access to this HW but hopefully this works. Only compile
> > > > tested.
> > > >
> > > > [1] https://lkml.org/lkml/2019/6/25/900
> > > >
> > > > v1 -> v2:
> > > > - rebased on top of v5.3-rc1 and adjusted to the recent changes from
> > > > Andy
> > > > - added additional two patches with minor improvements
> > > >
> > > > v2 -> v3:
> > > > - in patch 7/7: used initializers to set values for pdata and dev local
> > > > vars
> > > >
> > > > v3 -> v4:
> > > > - rebased on top of v5.4-rc1
> > > > - removed changes that are no longer relevant after commit ec665b756e6f
> > > > ("backlight: gpio-backlight: Correct initial power state handling")
> > > > - added patch 7/7
> > > >
> > > > v4 -> v5:
> > > > - in patch 7/7: added a comment replacing the name of the function being
> > > > pulled into probe()
> > > >
> > > > v5 -> v6:
> > > > - added a patch making the driver explicitly set the direction of the
> > > > GPIO
> > > > to output
> > > > - added a patch removing a redundant newline
> > > >
> > > > v6 -> v7:
> > > > - renamed the function calculating the new GPIO value for status update
> > > > - collected more tags
> > > >
> > > > Bartosz Golaszewski (9):
> > > > backlight: gpio: remove unneeded include
> > > > backlight: gpio: remove stray newline
> > > > backlight: gpio: explicitly set the direction of the GPIO
> > > > sh: ecovec24: add additional properties to the backlight device
> > > > backlight: gpio: simplify the platform data handling
> > > > sh: ecovec24: don't set unused fields in platform data
> > > > backlight: gpio: remove unused fields from platform data
> > > > backlight: gpio: use a helper variable for &pdev->dev
> > > > backlight: gpio: pull gpio_backlight_initial_power_state() into probe
> > > >
> > > > arch/sh/boards/mach-ecovec24/setup.c | 33 +++--
> > > > drivers/video/backlight/gpio_backlight.c | 128 +++
> > > > include/linux/platform_data/gpio_backlight.h | 3 -
> > > > 3 files changed, 69 insertions(+), 95 deletions(-)
> > > >
> > > >
> > >
> > > Lee, Daniel, Jingoo,
> > >
> > > Jacopo is travelling until November 1st and won't be able to test this
> > > again before this date. Do you think you can pick it up and in case
> > > anything's broken on SH, we can fix it after v5.5-rc1, so that it
> > > doesn't miss another merge window?
>
> November 1st (-rc6) will be fine.
>
> I'd rather apply it late-tested than early-non-tested.
>
> Hopefully Jacopo can prioritise testing this on Thursday or Friday,
> since Monday will be -rc7 which is really cutting it fine.
I'll do my best, I'll get home Friday late afternoon :)
>
> > Outside of holidays and other emergencies Lee usually collects up the
> > patches for backlight. I'm not sure when he plans to close things for
> > v5.5.
>
> In special cases such as these I can remain flexible.
>
> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Cleanup: replace prefered with preferred
On 10/23/19 4:56 AM, Jarkko Sakkinen wrote: On Tue, Oct 22, 2019 at 02:41:45PM -0700, Mark Salyzyn wrote: Replace all occurrences of prefered with preferred to make future checkpatch.pl's happy. A few places the incorrect spelling is matched with the correct spelling to preserve existing user space API. Signed-off-by: Mark Salyzyn I'd fix such things when the code is otherwise change and scope this patch only to Documentation/. There is no pragmatic benefit of doing this for the code. /Jarkko The pragmatic benefit comes with the use of an ABI/API checker (which is a 'distro' thing, not a top of tree kernel thing) produces its map which is typically required to be co-located in the same tree as the kernel repository. Quite a few ABI/API update checkins result in a checkpatch.pl complaint about the misspelled elements being (re-)recorded due to proximity. We have a separate task to improve how it is tracked in Android to reduce milepost marker changes that result in sweeping changes to the database which would reduce the occurrences. I will split this between pure and inert documentation/comments for now, with a followup later for the code portion which understandably is more controversial. Cleanup is the least appreciated part of kernel maintenance ;-}. Sincerely -- Mark Salyzyn ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Proposal to report GPU private memory allocations with sysfs nodes
Hi folks, This is Yiwei from the Android Platform Graphics team. On the downstream Android, vendors used to report GPU private memory allocations with debugfs nodes in their own formats. However, debugfs nodes are getting deprecated in the next Android release, so we are taking the chance to unify all the vendors to migrate their existing debugfs nodes into a standardized sysfs node structure. Then the platform is able to do a bunch of useful things: memory profiling, system health coverage, field metrics, local shell dump, in-app api, etc. Some vendors tend to do a lot of upstreams, so we are also seeking the upstream possibilities here instead of making it an Android only thing. Attached are screenshots for the node structure we drafted and an example for that. For the top level root, vendors can choose their own names based on the value of ro.gfx.sysfs.0 the vendors set. - For the multiple gpu driver cases, we can use ro.gfx.sysfs.1, ro.gfx.sysfs.2 for the 2nd and 3rd KMDs. - It's also allowed to put some sub-dir for example "kgsl/gpu_mem" or "mali0/gpu_mem" in the ro.gfx.sysfs. property if the root name under /sys/devices/ is already created and used for other purposes. For the 2nd level pids, there are usually just a couple of them per snapshot, since we only takes snapshot for the active ones. For the 3rd level types, the type name will be one of the GPU memory object types in lower case, and the value will be a comma separated sequence of size values for all the allocations under that specific type. - For the GPU memory object types, we defined 9 different types for Android: - // not accounted for in any other category UNKNOWN = 0; // shader binaries SHADER = 1; // allocations which have a lifetime similar to a VkCommandBuffer COMMAND = 2; // backing for VkDeviceMemory VULKAN = 3; // GL Texture and RenderBuffer GL_TEXTURE = 4; // GL Buffer GL_BUFFER = 5; // backing for query QUERY = 6; // allocations which have a lifetime similar to a VkDescriptorSet DESCRIPTOR = 7; // random transient things that the driver needs TRANSIENT = 8; - We are wondering if those type enumerations make sense to the upstream side as well, or maybe we just deal with our own different type sets. Cuz on the Android side, we'll just read those nodes named after the types we defined in the sysfs node structure. - The node value can be: 4096,81920,...,4096 Looking forward to any concerns/comments/suggestions! Best regards, Yiwei ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/gpu: Add comment for memory barrier
-Add comment for memory barrier
-Issue found using checkpatch.pl
Signed-off-by: Bhanusree
---
v2:modified memory barrier comments appropriately
drivers/gpu/drm/drm_cache.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 3bd76e9..e574261 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -62,10 +62,10 @@ static void drm_cache_flush_clflush(struct page *pages[],
{
unsigned long i;
- mb();
+ mb(); /*Full memory barrier used before so that CLFLUSH is ordered*/
for (i = 0; i < num_pages; i++)
drm_clflush_page(*pages++);
- mb();
+ mb(); /*Also used after CLFLUSH so that all cache is flushed*/
}
#endif
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] backlight: pwm_bl: fix cie1913 comments and constant
On 14/10/2019 09.27, Lee Jones wrote: > Applied, thanks. I'm not seeing the series in next-20191023, should it be there? Rasmus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dc.c:use kzalloc without test
dc.c:583:null check is needed after using kzalloc function
Signed-off-by: zhongshiqi
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 5d1aded..4b8819c 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -580,6 +580,10 @@ static bool construct(struct dc *dc,
#ifdef CONFIG_DRM_AMD_DC_DCN2_0
// Allocate memory for the vm_helper
dc->vm_helper = kzalloc(sizeof(struct vm_helper), GFP_KERNEL);
+ if (!dc->vm_helper) {
+ dm_error("%s: failed to create dc->vm_helper\n", __func__);
+ goto fail;
+ }
#endif
memcpy(&dc->bb_overrides, &init_params->bb_overrides,
sizeof(dc->bb_overrides));
--
2.9.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/36] ARM: samsung platform cleanup
Hi Arnd, Krzysztof, On 23/10/2019 9:39 PM, Arnd Bergmann wrote: > On Wed, Oct 23, 2019 at 3:11 PM Krzysztof Kozlowski wrote: >> On Thu, Oct 10, 2019 at 10:28:02PM +0200, Arnd Bergmann wrote: >>> The contents are available for testing in >>> >>> git://kernel.org:/pub/scm/linux/kernel/git/arnd/playground.git >>> s3c-multiplatform >> When sending v2, can you Cc: >> >> Paweł Chmiel >> Lihua Yao >> (or Lihua Yao if outlook.com bounces) You could reach me by e-mail to Lihua Yao . Neither 163.com nor outlook.com e-mail server cooperates well with vger.kernel.org mail list. >> Sergio Prado >> Sylwester Nawrocki >> >> These are folks which to my knowledge had working S3C and S5P boards >> so maybe they could provide testing. > Ok, will do. I've uploaded the modified version based on your comments to > the above URL for now. > > I'll probably give it a little more time before resending, but they > could already > start testing that version. Nice！ Sadly I am on vocation and will test it until Nov 15. Best Regards Lihua ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hmm 00/15] Consolidate the mmu notifier interval_tree and locking
On Wed, Oct 23, 2019 at 12:52:23PM -0400, Jerome Glisse wrote: > > Going another step further what hinders us to put the lock into the mmu > > range notifier itself and have _lock()/_unlock() helpers? > > > > I mean having the lock in the driver only makes sense when the driver would > > be using the same lock for multiple things, e.g. multiple MMU range > > notifiers under the same lock. But I really don't see that use case here. > > I actualy do, nouveau use one lock to protect the page table and that's the > lock that matter. You can have multiple range for a single page table, idea > being only a sub-set of the process address space is ever accessed by the > GPU and those it is better to focus on this sub-set and track invalidation in > a finer grain. mlx5 is similar, but not currently coded quite right, there is one lock that protects the command queue for submitting invalidations to the HW and it doesn't make a lot of sense to have additional fine grained locking beyond that. So I suppose the intent here that most drivers would have a single 'page table lock' that protects the HW's page table update, and this lock is the one that should be held while upating and checking the sequence number. dma_fence based drivers are possibly a little different, I think they can just use a spinlock, their pattern should probably be something like fault: hmm_range_fault() spin_lock() if (mmu_range_read_retry())) goto again dma_fence_init(mrn->fence) spin_unlock() invalidate: spin_lock() is_inited = 'dma fence init has been called' spin_unlock() if (is_inited) dma_fence_wait(fence) I'm not sure, never used dma_fence before. The key thing is that the dma_fence_wait() cannot block until after the mmu_range_read_retry() & unlock completes. Otherwise it can deadlock with hmm_range_fault(). It would be nice to figure this out and add it to the hmm.rst as we do have two drivers using the dma_fence scheme. Also, the use of a spinlock here probably says we should keep the lock external. But, it sounds like the mmu_range_notifier_update_seq() is a good idea, so let me add that in v2. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 1/5] drm/dsi: clean up DSI data type definitions
Looks good to me.
Reviewed-by: Vandita Kulkarni
> -Original Message-
> From: Jani Nikula
> Sent: Tuesday, October 22, 2019 3:40 PM
> To: dri-devel@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org; Nikula, Jani ;
> Kulkarni, Vandita
> Subject: [PATCH 1/5] drm/dsi: clean up DSI data type definitions
>
> Rename picture parameter set (it's a long packet, not a long write) and
> compression mode (it's not a DCS command) enumerations according to the
> DSI specification. Order the types according to the spec. Use tabs instead of
> spaces for indentation. Use all lower case for hex.
>
> Cc: Vandita Kulkarni
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 4 ++--
> include/video/mipi_display.h | 10 +-
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c
> b/drivers/gpu/drm/drm_mipi_dsi.c index bd2498bbd74a..f237d80828c3
> 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -373,6 +373,7 @@ bool mipi_dsi_packet_format_is_short(u8 type)
> case MIPI_DSI_V_SYNC_END:
> case MIPI_DSI_H_SYNC_START:
> case MIPI_DSI_H_SYNC_END:
> + case MIPI_DSI_COMPRESSION_MODE:
> case MIPI_DSI_END_OF_TRANSMISSION:
> case MIPI_DSI_COLOR_MODE_OFF:
> case MIPI_DSI_COLOR_MODE_ON:
> @@ -387,7 +388,6 @@ bool mipi_dsi_packet_format_is_short(u8 type)
> case MIPI_DSI_DCS_SHORT_WRITE:
> case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
> case MIPI_DSI_DCS_READ:
> - case MIPI_DSI_DCS_COMPRESSION_MODE:
> case MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE:
> return true;
> }
> @@ -406,11 +406,11 @@
> EXPORT_SYMBOL(mipi_dsi_packet_format_is_short);
> bool mipi_dsi_packet_format_is_long(u8 type) {
> switch (type) {
> - case MIPI_DSI_PPS_LONG_WRITE:
> case MIPI_DSI_NULL_PACKET:
> case MIPI_DSI_BLANKING_PACKET:
> case MIPI_DSI_GENERIC_LONG_WRITE:
> case MIPI_DSI_DCS_LONG_WRITE:
> + case MIPI_DSI_PICTURE_PARAMETER_SET:
> case MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20:
> case MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24:
> case MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16:
> diff --git a/include/video/mipi_display.h b/include/video/mipi_display.h
> index cba57a678daf..79fd71cf4934 100644
> --- a/include/video/mipi_display.h
> +++ b/include/video/mipi_display.h
> @@ -17,6 +17,9 @@ enum {
> MIPI_DSI_H_SYNC_START = 0x21,
> MIPI_DSI_H_SYNC_END = 0x31,
>
> + MIPI_DSI_COMPRESSION_MODE = 0x07,
> + MIPI_DSI_END_OF_TRANSMISSION= 0x08,
> +
> MIPI_DSI_COLOR_MODE_OFF = 0x02,
> MIPI_DSI_COLOR_MODE_ON = 0x12,
> MIPI_DSI_SHUTDOWN_PERIPHERAL= 0x22,
> @@ -35,18 +38,15 @@ enum {
>
> MIPI_DSI_DCS_READ = 0x06,
>
> - MIPI_DSI_DCS_COMPRESSION_MODE = 0x07,
> - MIPI_DSI_PPS_LONG_WRITE = 0x0A,
> -
> MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE =
> 0x37,
>
> - MIPI_DSI_END_OF_TRANSMISSION= 0x08,
> -
> MIPI_DSI_NULL_PACKET= 0x09,
> MIPI_DSI_BLANKING_PACKET= 0x19,
> MIPI_DSI_GENERIC_LONG_WRITE = 0x29,
> MIPI_DSI_DCS_LONG_WRITE = 0x39,
>
> + MIPI_DSI_PICTURE_PARAMETER_SET = 0x0a,
> +
> MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20= 0x0c,
> MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24= 0x1c,
> MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16= 0x2c,
> --
> 2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline
On Thu, Oct 17, 2019 at 3:22 PM Maxime Ripard wrote:
>
> On Wed, Oct 16, 2019 at 02:19:44PM +0530, Jagan Teki wrote:
> > On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard wrote:
> > >
> > > On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> > > > On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard wrote:
> > > > >
> > > > > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > > > > Add MIPI DSI pipeline for Allwinner A64.
> > > > > >
> > > > > > - dsi node, with A64 compatible since it doesn't support
> > > > > > DSI_SCLK gating unlike A33
> > > > > > - dphy node, with A64 compatible with A33 fallback since
> > > > > > DPHY on A64 and A33 is similar
> > > > > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > > > > >
> > > > > > Signed-off-by: Jagan Teki
> > > > > > Tested-by: Merlijn Wajer
> > > > > > ---
> > > > > > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38
> > > > > > +++
> > > > > > 1 file changed, 38 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > index 69128a6dfc46..ad4170b8aee0 100644
> > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > @@ -382,6 +382,12 @@
> > > > > > #address-cells = <1>;
> > > > > > #size-cells = <0>;
> > > > > > reg = <1>;
> > > > > > +
> > > > > > + tcon0_out_dsi: endpoint@1 {
> > > > > > + reg = <1>;
> > > > > > + remote-endpoint =
> > > > > > <&dsi_in_tcon0>;
> > > > > > +
> > > > > > allwinner,tcon-channel = <1>;
> > > > > > + };
> > > > > > };
> > > > > > };
> > > > > > };
> > > > > > @@ -1003,6 +1009,38 @@
> > > > > > status = "disabled";
> > > > > > };
> > > > > >
> > > > > > + dsi: dsi@1ca {
> > > > > > + compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > > > > + reg = <0x01ca 0x1000>;
> > > > > > + interrupts = ;
> > > > > > + clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > > > > + clock-names = "bus";
> > > > >
> > > > > This won't validate with the bindings you have either here, since it
> > > > > still expects bus and mod.
> > > > >
> > > > > I guess in that cas, we can just drop clock-names, which will require
> > > > > a bit of work on the driver side as well.
> > > >
> > > > Okay.
> > > > mod clock is not required for a64, ie reason we have has_mod_clk quirk
> > > > patch. Adjust the clock-names: on dt-bindings would make sense here,
> > > > what do you think?
> > >
> > > I'm confused, what are you suggesting?
> >
> > Sorry for the confusion.
> >
> > The mod clock is not required for A64 and we have a patch for handling
> > mod clock using has_mod_clk quirk(on the series), indeed the mod clock
> > is available in A31 and not needed for A64. So, to satisfy this
> > requirement the clock-names on dt-bindings can update to make mod
> > clock-name is optional and bus clock is required.
>
> No, the bus clock name is not needed if there's only one clock.
Okay, is it because the same clock handle it on PHY side?
>
> > I'm not exactly sure, this is correct but trying to understand if it
> > is possible or not? something like
> >
> >clocks:
> > minItems: 1
> > maxItems: 2
> > items:
> >- description: Bus Clock
> >- description: Module Clock
>
> That's correct.
>
> >clock-names:
> > minItems: 1
> > maxItems: 2
> > items:
> >- const: bus
> >- const: mod
>
> Here, just keep the current clock-names definition, and make it
> required only for SoCs that are not the A64
Okay, please have a look here I have pasted the diff for comments.
clocks:
+minItems: 2
items:
- description: Bus Clock
- description: Module Clock
@@ -64,14 +65,26 @@ required:
- compatible
- reg
- interrupts
- - clocks
- - clock-names
- phys
- phy-names
- resets
- vcc-dsi-supply
- port
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: allwinner,sun6i-a31-mipi-dsi
+ then:
+properties:
+ clocks:
+minItems: 2
+required:
+ - clocks
+ - clock-names
+
additionalProperties: false
I have marked minItems: 2 on clocks since we need to use minimum of 2
clocks like both bus and mod not mod clock alone.
Please let me know your comments.
Jagan.
___
Re: [RFC,3/3] drm/komeda: Allow non-component drm_bridge only endpoints
Hi Russell, On Tuesday, 22 October 2019 15:53:29 BST Russell King - ARM Linux admin wrote: > On Tue, Oct 22, 2019 at 03:42:07PM +0100, Russell King - ARM Linux admin > wrote: > > On Tue, Oct 22, 2019 at 10:50:42AM +0200, Daniel Vetter wrote: > > > On Tue, Oct 22, 2019 at 10:48 AM Russell King - ARM Linux admin > > > wrote: > > > > I had a patches, which is why I raised the problem with the core: > > > > > > > > 6961edfee26d bridge hacks using device links > > > > > > > > but it never went further than an experiment at the time because of the > > > > problems in the core. As it was a hack, it never got posted. Seems > > > > that kernel tree (for the cubox) is still 5.2 based, so has a lot of > > > > patches and might need updating to a more recent base before anything > > > > can be tested. > > > > > > > > > For reference, the panel patch: > > > > > > https://patchwork.kernel.org/patch/10364873/ > > > > > > And the huge discussion around bridges, that resulted in Rafael > > > Wyzocki fixing all the core issues: > > > > > > https://www.spinics.net/lists/dri-devel/msg201927.html > > > > > > James, do you want to look into this for bridges? > > > > I can now confirm that it does work. > > Something like this - it's based off an older kernel, so may be missing > some of the bridge drivers, but should be sufficient for people to test > with. Thanks for the patch, I tested to the limit that our driver allows at the moment -- rmmod'ing the bridge while the driver is not in use -- and I don't see any issues there. komeda successfully gets removed then re-probed once the bridge reappears. For that part, Tested-by: Mihail Atanassov I couldn't sadly check a hot unplug since we have an mm bug or two that I need to sort out first. If anyone else has a hot-unplug capable driver and can test, it'd be good to ensure that also functions properly. > > 8< > From: Russell King > Subject: [PATCH] drm/bridge: add support for device links to bridge > > Bridge devices have been a potential for kernel oops as their lifetime > is independent of the DRM device that they are bound to. Hence, if a > bridge device is unbound while the parent DRM device is using them, the > parent happily continues to use the bridge device, calling the driver > and accessing its objects that have been freed. > > This can cause kernel memory corruption and kernel oops. > > To control this, use device links to ensure that the parent DRM device > is unbound when the bridge device is unbound, and when the bridge > device is re-bound, automatically rebind the parent DRM device. > > Signed-off-by: Russell King > --- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 1 + > drivers/gpu/drm/bridge/analogix-anx78xx.c | 1 + > drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 + > drivers/gpu/drm/bridge/lvds-encoder.c | 1 + > .../bridge/megachips-stdp-ge-b850v3-fw.c | 1 + > drivers/gpu/drm/bridge/nxp-ptn3460.c | 1 + > drivers/gpu/drm/bridge/panel.c| 1 + > drivers/gpu/drm/bridge/parade-ps8622.c| 1 + > drivers/gpu/drm/bridge/sii902x.c | 1 + > drivers/gpu/drm/bridge/sii9234.c | 1 + > drivers/gpu/drm/bridge/sil-sii8620.c | 1 + > drivers/gpu/drm/bridge/tc358767.c | 1 + > drivers/gpu/drm/bridge/ti-tfp410.c| 1 + > drivers/gpu/drm/drm_bridge.c | 48 ++- > drivers/gpu/drm/i2c/tda998x_drv.c | 1 + > include/drm/drm_bridge.h | 4 ++ > 16 files changed, 53 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index f6d2681f6927..6a5906da58ea 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -1217,6 +1217,7 @@ static int adv7511_probe(struct i2c_client *i2c, const > struct i2c_device_id *id) > goto err_unregister_cec; > > adv7511->bridge.funcs = &adv7511_bridge_funcs; > + adv7511->bridge.device = dev; > adv7511->bridge.of_node = dev->of_node; > > drm_bridge_add(&adv7511->bridge); > diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c > b/drivers/gpu/drm/bridge/analogix-anx78xx.c > index 3c7cc5af735c..77ff17c38037 100644 > --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c > +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c > @@ -1323,6 +1323,7 @@ static int anx78xx_i2c_probe(struct i2c_client *client, > > mutex_init(&anx78xx->lock); > > + anx78xx->bridge.device = &client->dev; > #if IS_ENABLED(CONFIG_OF) > anx78xx->bridge.of_node = client->dev.of_node; > #endif > diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c > b/drivers/gpu/drm/bridge/dumb-vga-dac.c > index d32885b906ae..40169920560e 100644 > --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c > +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c > @@ -202,6 +202,7 @@ static int dumb_vga_probe(
[PATCH v2 1/4] drm/vram-helpers: Add helpers for prepare_fb() and cleanup_fb()
The new helpers pin and unpin a framebuffer's GEM VRAM objects during
plane updates. This should be sufficient for most drivers' implementation
of prepare_fb() and cleanup_fb().
v2:
* provide helpers for struct drm_simple_display_pipe_funcs
* rename plane-helper funcs
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_vram_helper.c | 126 ++
include/drm/drm_gem_vram_helper.h | 25 +
2 files changed, 151 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index b86fe0fa9d05..7fe93cd38eea 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -3,10 +3,13 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
#include
+#include
#include
static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
@@ -646,6 +649,129 @@ int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file
*file,
}
EXPORT_SYMBOL(drm_gem_vram_driver_dumb_mmap_offset);
+/*
+ * Helpers for struct drm_plane_helper_funcs
+ */
+
+/**
+ * drm_gem_vram_plane_helper_prepare_fb() - \
+ * Implements &struct drm_plane_helper_funcs.prepare_fb
+ * @plane: a DRM plane
+ * @new_state: the plane's new state
+ *
+ * During plane updates, this function pins the GEM VRAM
+ * objects of the plane's new framebuffer to VRAM. Call
+ * drm_gem_vram_plane_helper_cleanup_fb() to unpin them.
+ *
+ * Returns:
+ * 0 on success, or
+ * a negative errno code otherwise.
+ */
+int
+drm_gem_vram_plane_helper_prepare_fb(struct drm_plane *plane,
+struct drm_plane_state *new_state)
+{
+ size_t i;
+ struct drm_gem_vram_object *gbo;
+ int ret;
+
+ if (!new_state->fb)
+ return 0;
+
+ for (i = 0; i < ARRAY_SIZE(new_state->fb->obj); ++i) {
+ if (!new_state->fb->obj[i])
+ continue;
+ gbo = drm_gem_vram_of_gem(new_state->fb->obj[i]);
+ ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
+ if (ret)
+ goto err_drm_gem_vram_unpin;
+ }
+
+ return 0;
+
+err_drm_gem_vram_unpin:
+ while (i) {
+ --i;
+ gbo = drm_gem_vram_of_gem(new_state->fb->obj[i]);
+ drm_gem_vram_unpin(gbo);
+ }
+ return ret;
+}
+EXPORT_SYMBOL(drm_gem_vram_plane_helper_prepare_fb);
+
+/**
+ * drm_gem_vram_plane_helper_cleanup_fb() - \
+ * Implements &struct drm_plane_helper_funcs.cleanup_fb
+ * @plane: a DRM plane
+ * @old_state: the plane's old state
+ *
+ * During plane updates, this function unpins the GEM VRAM
+ * objects of the plane's old framebuffer from VRAM. Complements
+ * drm_gem_vram_plane_helper_prepare_fb().
+ */
+void
+drm_gem_vram_plane_helper_cleanup_fb(struct drm_plane *plane,
+struct drm_plane_state *old_state)
+{
+ size_t i;
+ struct drm_gem_vram_object *gbo;
+
+ if (!old_state->fb)
+ return;
+
+ for (i = 0; i < ARRAY_SIZE(old_state->fb->obj); ++i) {
+ if (!old_state->fb->obj[i])
+ continue;
+ gbo = drm_gem_vram_of_gem(old_state->fb->obj[i]);
+ drm_gem_vram_unpin(gbo);
+ }
+}
+EXPORT_SYMBOL(drm_gem_vram_plane_helper_cleanup_fb);
+
+/*
+ * Helpers for struct drm_simple_display_pipe_funcs
+ */
+
+/**
+ * drm_gem_vram_simple_display_pipe_prepare_fb() - \
+ * Implements &struct drm_simple_display_pipe_funcs.prepare_fb
+ * @pipe: a simple display pipe
+ * @new_state: the plane's new state
+ *
+ * During plane updates, this function pins the GEM VRAM
+ * objects of the plane's new framebuffer to VRAM. Call
+ * drm_gem_vram_simple_display_pipe_cleanup_fb() to unpin them.
+ *
+ * Returns:
+ * 0 on success, or
+ * a negative errno code otherwise.
+ */
+int drm_gem_vram_simple_display_pipe_prepare_fb(
+ struct drm_simple_display_pipe *pipe,
+ struct drm_plane_state *new_state)
+{
+ return drm_gem_vram_plane_helper_prepare_fb(&pipe->plane, new_state);
+}
+EXPORT_SYMBOL(drm_gem_vram_simple_display_pipe_prepare_fb);
+
+/**
+ * drm_gem_vram_simple_display_pipe_cleanup_fb() - \
+ * Implements &struct drm_simple_display_pipe_funcs.cleanup_fb
+ * @pipe: a simple display pipe
+ * @old_state: the plane's old state
+ *
+ * During plane updates, this function unpins the GEM VRAM
+ * objects of the plane's old framebuffer from VRAM. Complements
+ * drm_gem_vram_simple_display_pipe_prepare_fb().
+ */
+void drm_gem_vram_simple_display_pipe_cleanup_fb(
+ struct drm_simple_display_pipe *pipe,
+ struct drm_plane_state *old_state)
+{
+ drm_gem_vram_plane_helper_cleanup_fb(&pipe->plane, old_state);
+}
+EXPORT_SYMBOL(drm_gem_vram_simple_display_pipe_cleanup_fb);
+
/*
* PRIME helpers
*/
diff --git a/include/drm/drm_gem_vram_helper.h
b/incl
[PATCH v2 3/4] drm/hisilicon/hibmc: Use GEM VRAM's prepare_fb() and cleanup_fb() helpers
This patch implements prepare_fb() and cleanup_fb() in hibmc with the
GEM VRAM helpers. In the current code, pinning the BO is performed by
hibmc_plane_atomic_update(), where the operation does not belong.
This patch also fixes a bug where the pinned BO was never unpinned.
Pinning multiple BOs would have exhaused the available VRAM and further
pin operations would have failed, leaving the display in a corrupt
state.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
index cc4c41748cfb..6527a97f68a3 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
@@ -96,7 +96,6 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
{
struct drm_plane_state *state = plane->state;
u32 reg;
- int ret;
s64 gpu_addr = 0;
unsigned int line_l;
struct hibmc_drm_private *priv = plane->dev->dev_private;
@@ -109,16 +108,9 @@ static void hibmc_plane_atomic_update(struct drm_plane
*plane,
hibmc_fb = to_hibmc_framebuffer(state->fb);
gbo = drm_gem_vram_of_gem(hibmc_fb->obj);
- ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
- if (ret) {
- DRM_ERROR("failed to pin bo: %d", ret);
- return;
- }
gpu_addr = drm_gem_vram_offset(gbo);
- if (gpu_addr < 0) {
- drm_gem_vram_unpin(gbo);
- return;
- }
+ if (WARN_ON_ONCE(gpu_addr < 0))
+ return; /* Bug: we didn't pin the BO to VRAM in prepare_fb. */
writel(gpu_addr, priv->mmio + HIBMC_CRT_FB_ADDRESS);
@@ -157,6 +149,8 @@ static struct drm_plane_funcs hibmc_plane_funcs = {
};
static const struct drm_plane_helper_funcs hibmc_plane_helper_funcs = {
+ .prepare_fb = drm_gem_vram_plane_helper_prepare_fb,
+ .cleanup_fb = drm_gem_vram_plane_helper_cleanup_fb,
.atomic_check = hibmc_plane_atomic_check,
.atomic_update = hibmc_plane_atomic_update,
};
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/4] drm/vboxvideo: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers
GEM VRAM provides an implementation for prepare_fb() and cleanup_fb()
of struct drm_plane_helper_funcs. Switch over vboxvideo.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/vboxvideo/vbox_mode.c | 61 ++-
1 file changed, 4 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c
b/drivers/gpu/drm/vboxvideo/vbox_mode.c
index b5604d32122e..cea38c5345c6 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
@@ -334,35 +334,6 @@ static void vbox_primary_atomic_disable(struct drm_plane
*plane,
old_state->src_y >> 16);
}
-static int vbox_primary_prepare_fb(struct drm_plane *plane,
- struct drm_plane_state *new_state)
-{
- struct drm_gem_vram_object *gbo;
- int ret;
-
- if (!new_state->fb)
- return 0;
-
- gbo = drm_gem_vram_of_gem(new_state->fb->obj[0]);
- ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
- if (ret)
- DRM_WARN("Error %d pinning new fb, out of video mem?\n", ret);
-
- return ret;
-}
-
-static void vbox_primary_cleanup_fb(struct drm_plane *plane,
- struct drm_plane_state *old_state)
-{
- struct drm_gem_vram_object *gbo;
-
- if (!old_state->fb)
- return;
-
- gbo = drm_gem_vram_of_gem(old_state->fb->obj[0]);
- drm_gem_vram_unpin(gbo);
-}
-
static int vbox_cursor_atomic_check(struct drm_plane *plane,
struct drm_plane_state *new_state)
{
@@ -492,30 +463,6 @@ static void vbox_cursor_atomic_disable(struct drm_plane
*plane,
mutex_unlock(&vbox->hw_mutex);
}
-static int vbox_cursor_prepare_fb(struct drm_plane *plane,
- struct drm_plane_state *new_state)
-{
- struct drm_gem_vram_object *gbo;
-
- if (!new_state->fb)
- return 0;
-
- gbo = drm_gem_vram_of_gem(new_state->fb->obj[0]);
- return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_SYSTEM);
-}
-
-static void vbox_cursor_cleanup_fb(struct drm_plane *plane,
- struct drm_plane_state *old_state)
-{
- struct drm_gem_vram_object *gbo;
-
- if (!plane->state->fb)
- return;
-
- gbo = drm_gem_vram_of_gem(plane->state->fb->obj[0]);
- drm_gem_vram_unpin(gbo);
-}
-
static const u32 vbox_cursor_plane_formats[] = {
DRM_FORMAT_ARGB,
};
@@ -524,8 +471,8 @@ static const struct drm_plane_helper_funcs
vbox_cursor_helper_funcs = {
.atomic_check = vbox_cursor_atomic_check,
.atomic_update = vbox_cursor_atomic_update,
.atomic_disable = vbox_cursor_atomic_disable,
- .prepare_fb = vbox_cursor_prepare_fb,
- .cleanup_fb = vbox_cursor_cleanup_fb,
+ .prepare_fb = drm_gem_vram_plane_helper_prepare_fb,
+ .cleanup_fb = drm_gem_vram_plane_helper_cleanup_fb,
};
static const struct drm_plane_funcs vbox_cursor_plane_funcs = {
@@ -546,8 +493,8 @@ static const struct drm_plane_helper_funcs
vbox_primary_helper_funcs = {
.atomic_check = vbox_primary_atomic_check,
.atomic_update = vbox_primary_atomic_update,
.atomic_disable = vbox_primary_atomic_disable,
- .prepare_fb = vbox_primary_prepare_fb,
- .cleanup_fb = vbox_primary_cleanup_fb,
+ .prepare_fb = drm_gem_vram_plane_helper_prepare_fb,
+ .cleanup_fb = drm_gem_vram_plane_helper_cleanup_fb,
};
static const struct drm_plane_funcs vbox_primary_plane_funcs = {
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/4] drm/bochs: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers
GEM VRAM provides an implementation for prepare_fb() and cleanup_fb()
of struct drm_simple_display_pipe_funcs. Switch over bochs.
v2:
* use helpers for struct drm_simple_display_pipe_funcs
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/bochs/bochs_kms.c | 26 ++
1 file changed, 2 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
b/drivers/gpu/drm/bochs/bochs_kms.c
index 02a9c1ed165b..3f0006c2470d 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -69,33 +69,11 @@ static void bochs_pipe_update(struct
drm_simple_display_pipe *pipe,
}
}
-static int bochs_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
-struct drm_plane_state *new_state)
-{
- struct drm_gem_vram_object *gbo;
-
- if (!new_state->fb)
- return 0;
- gbo = drm_gem_vram_of_gem(new_state->fb->obj[0]);
- return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
-}
-
-static void bochs_pipe_cleanup_fb(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *old_state)
-{
- struct drm_gem_vram_object *gbo;
-
- if (!old_state->fb)
- return;
- gbo = drm_gem_vram_of_gem(old_state->fb->obj[0]);
- drm_gem_vram_unpin(gbo);
-}
-
static const struct drm_simple_display_pipe_funcs bochs_pipe_funcs = {
.enable = bochs_pipe_enable,
.update = bochs_pipe_update,
- .prepare_fb = bochs_pipe_prepare_fb,
- .cleanup_fb = bochs_pipe_cleanup_fb,
+ .prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb,
+ .cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb,
};
static int bochs_connector_get_modes(struct drm_connector *connector)
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #150 from Stijn Tintel --- (In reply to Jaap Buurman from comment #142) > How can I set both AMD_DEBUG=nongg and AMD_DEBUG=nodma in the > /etc/environment file? Do they need to be on two separate lines, or will the > second line simply overwrite the first one by setting the same environment > variable? Do they need to be comma separated maybe? AMD_DEBUG="nodma nongg" I've been running like this since I found this bug report. Current uptime: 11:08:41 up 4 days, 4:12, 11 users, load average: 8,56, 8,33, 8,15 Haven't experienced a single hang, not even a kernel oops. Before that, the system was frustratingly unstable. If you need stability, put this in /etc/environment (or /etc/env.d/99amdgpu or so if your distro supports /etc/env.d). Running on Gentoo, kernel 5.3.4, mesa 19.2.1, llvm 9.0.0, libdrm 2.4.99, xf86-video-amdgpu git e6fce59a071220967fcd4e2c9e4a262c72870761. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/4] drm/vram: Provide helpers for prepare_fb() and cleanup_fb()
The implementation of the plane's call-back functions prepare_fb() and cleanup_fb() for GEM VRAM helpers are sharable among drivers. Patch #3 also fixes two bugs that have been present in hibmc since it was first added. The primary plane's atomic_update() is not responsible for pinning BOs. And it never unpinned unused BOs. VRAM is being exausted over time. The new helpers have been tested to work. v2: * provide helpers for struct drm_simple_display_pipe_funcs, and... * ...use them in bochs Thomas Zimmermann (4): drm/vram-helpers: Add helpers for prepare_fb() and cleanup_fb() drm/bochs: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers drm/hisilicon/hibmc: Use GEM VRAM's prepare_fb() and cleanup_fb() helpers drm/vboxvideo: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers drivers/gpu/drm/bochs/bochs_kms.c | 26 +--- drivers/gpu/drm/drm_gem_vram_helper.c | 126 ++ .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 14 +- drivers/gpu/drm/vboxvideo/vbox_mode.c | 61 + include/drm/drm_gem_vram_helper.h | 25 5 files changed, 161 insertions(+), 91 deletions(-) -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/etnaviv: reinstate MMUv1 command buffer window check
Hi Lucas,
On Wed, Oct 16, 2019 at 04:27:16PM +0200, Lucas Stach wrote:
> The switch to per-process address spaces erroneously dropped the check
> which validated that the command buffer is mapped through the linear
> apperture as required by the hardware. This turned a system
> misconfiguration with a helpful error message into a very hard to
> debug issue. Reinstate the check at the appropriate location.
>
> Fixes: 17e4660ae3d7 (drm/etnaviv: implement per-process address spaces on
> MMUv2)
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 17 ++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> index 35ebae6a1be7..3607d348c298 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> @@ -328,12 +328,23 @@ etnaviv_iommu_context_init(struct etnaviv_iommu_global
> *global,
>
> ret = etnaviv_cmdbuf_suballoc_map(suballoc, ctx, &ctx->cmdbuf_mapping,
> global->memory_base);
> - if (ret) {
> - global->ops->free(ctx);
> - return NULL;
> + if (ret)
> + goto out_free;
> +
> + if (global->version == ETNAVIV_IOMMU_V1 &&
> + ctx->cmdbuf_mapping.iova > 0x8000) {
> + dev_err(global->dev,
> + "command buffer outside valid memory window\n");
> + goto out_unmap;
> }
>
> return ctx;
> +
> +out_unmap:
> + etnaviv_cmdbuf_suballoc_unmap(ctx, &ctx->cmdbuf_mapping);
> +out_free:
> + global->ops->free(ctx);
> + return NULL;
> }
>
> void etnaviv_iommu_restore(struct etnaviv_gpu *gpu,
Reviewed-by: Guido Günther
Cheers,
-- Guido
> --
> 2.20.1
>
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/nouveau: Move the declaration of struct nouveau_conn_atom up a bit
Place the declaration of struct nouveau_conn_atom above that of
struct nouveau_connector. This commit makes no changes to the moved
block what so ever, it just moves it up a bit.
This is a preparation patch to fix some issues with connector handling
on pre nv50 displays (which do not use atomic modesetting).
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/nouveau/nouveau_connector.h | 110 ++--
1 file changed, 55 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h
b/drivers/gpu/drm/nouveau/nouveau_connector.h
index f43a8d63aef8..de9588420884 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -29,6 +29,7 @@
#include
+#include
#include
#include
#include
@@ -44,6 +45,60 @@ struct dcb_output;
struct nouveau_backlight;
#endif
+#define nouveau_conn_atom(p)
\
+ container_of((p), struct nouveau_conn_atom, state)
+
+struct nouveau_conn_atom {
+ struct drm_connector_state state;
+
+ struct {
+ /* The enum values specifically defined here match nv50/gf119
+* hw values, and the code relies on this.
+*/
+ enum {
+ DITHERING_MODE_OFF = 0x00,
+ DITHERING_MODE_ON = 0x01,
+ DITHERING_MODE_DYNAMIC2X2 = 0x10 | DITHERING_MODE_ON,
+ DITHERING_MODE_STATIC2X2 = 0x18 | DITHERING_MODE_ON,
+ DITHERING_MODE_TEMPORAL = 0x20 | DITHERING_MODE_ON,
+ DITHERING_MODE_AUTO
+ } mode;
+ enum {
+ DITHERING_DEPTH_6BPC = 0x00,
+ DITHERING_DEPTH_8BPC = 0x02,
+ DITHERING_DEPTH_AUTO
+ } depth;
+ } dither;
+
+ struct {
+ int mode; /* DRM_MODE_SCALE_* */
+ struct {
+ enum {
+ UNDERSCAN_OFF,
+ UNDERSCAN_ON,
+ UNDERSCAN_AUTO,
+ } mode;
+ u32 hborder;
+ u32 vborder;
+ } underscan;
+ bool full;
+ } scaler;
+
+ struct {
+ int color_vibrance;
+ int vibrant_hue;
+ } procamp;
+
+ union {
+ struct {
+ bool dither:1;
+ bool scaler:1;
+ bool procamp:1;
+ };
+ u8 mask;
+ } set;
+};
+
struct nouveau_connector {
struct drm_connector base;
enum dcb_connector_type type;
@@ -121,61 +176,6 @@ extern int nouveau_ignorelid;
extern int nouveau_duallink;
extern int nouveau_hdmimhz;
-#include
-#define nouveau_conn_atom(p)
\
- container_of((p), struct nouveau_conn_atom, state)
-
-struct nouveau_conn_atom {
- struct drm_connector_state state;
-
- struct {
- /* The enum values specifically defined here match nv50/gf119
-* hw values, and the code relies on this.
-*/
- enum {
- DITHERING_MODE_OFF = 0x00,
- DITHERING_MODE_ON = 0x01,
- DITHERING_MODE_DYNAMIC2X2 = 0x10 | DITHERING_MODE_ON,
- DITHERING_MODE_STATIC2X2 = 0x18 | DITHERING_MODE_ON,
- DITHERING_MODE_TEMPORAL = 0x20 | DITHERING_MODE_ON,
- DITHERING_MODE_AUTO
- } mode;
- enum {
- DITHERING_DEPTH_6BPC = 0x00,
- DITHERING_DEPTH_8BPC = 0x02,
- DITHERING_DEPTH_AUTO
- } depth;
- } dither;
-
- struct {
- int mode; /* DRM_MODE_SCALE_* */
- struct {
- enum {
- UNDERSCAN_OFF,
- UNDERSCAN_ON,
- UNDERSCAN_AUTO,
- } mode;
- u32 hborder;
- u32 vborder;
- } underscan;
- bool full;
- } scaler;
-
- struct {
- int color_vibrance;
- int vibrant_hue;
- } procamp;
-
- union {
- struct {
- bool dither:1;
- bool scaler:1;
- bool procamp:1;
- };
- u8 mask;
- } set;
-};
-
void nouveau_conn_attach_properties(struct drm_connector *);
void nouveau_conn_reset(struct drm_connector *);
struct drm_connector_state *
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedeskto
[PATCH v2 2/2] drm/nouveau: Fix drm-core using atomic code-paths on pre-nv50 hardware
We do not support atomic modesetting on pre-nv50 hardware, but until now
our connector code was setting drm_connector->state on pre-nv50 hardware.
This causes the core to enter atomic modesetting paths in at least:
1. drm_connector_get_encoder(), returning connector->state->best_encoder
which is always 0, causing us to always report 0 as encoder_id in
the drmModeConnector struct returned by drmModeGetConnector().
2. drm_encoder_get_crtc(), returning NULL because uses_atomic get set,
causing us to always report 0 as crtc_id in the drmModeEncoder struct
returned by drmModeGetEncoder()
Which in turn confuses userspace, at least plymouth thinks that the pipe
has changed because of this and tries to reconfigure it unnecessarily.
More in general we should not set drm_connector->state in the non-atomic
code as this violates the drm-core's expectations.
This commit fixes this by using a nouveau_conn_atom struct embedded in the
nouveau_connector struct for property handling in the non-atomic case.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1706557
Signed-off-by: Hans de Goede
---
Changes in v2:
-Fix trailing whitespace issue
---
drivers/gpu/drm/nouveau/nouveau_connector.c | 28 +++--
drivers/gpu/drm/nouveau/nouveau_connector.h | 6 +
2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 94dfa2e5a9ab..805b341db165 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -245,14 +245,22 @@ nouveau_conn_atomic_duplicate_state(struct drm_connector
*connector)
void
nouveau_conn_reset(struct drm_connector *connector)
{
+ struct nouveau_connector *nv_connector = nouveau_connector(connector);
struct nouveau_conn_atom *asyc;
- if (WARN_ON(!(asyc = kzalloc(sizeof(*asyc), GFP_KERNEL
- return;
+ if (drm_drv_uses_atomic_modeset(connector->dev)) {
+ if (WARN_ON(!(asyc = kzalloc(sizeof(*asyc), GFP_KERNEL
+ return;
+
+ if (connector->state)
+ nouveau_conn_atomic_destroy_state(connector,
+ connector->state);
+
+ __drm_atomic_helper_connector_reset(connector, &asyc->state);
+ } else {
+ asyc = &nv_connector->properties_state;
+ }
- if (connector->state)
- nouveau_conn_atomic_destroy_state(connector, connector->state);
- __drm_atomic_helper_connector_reset(connector, &asyc->state);
asyc->dither.mode = DITHERING_MODE_AUTO;
asyc->dither.depth = DITHERING_DEPTH_AUTO;
asyc->scaler.mode = DRM_MODE_SCALE_NONE;
@@ -276,8 +284,14 @@ void
nouveau_conn_attach_properties(struct drm_connector *connector)
{
struct drm_device *dev = connector->dev;
- struct nouveau_conn_atom *armc = nouveau_conn_atom(connector->state);
struct nouveau_display *disp = nouveau_display(dev);
+ struct nouveau_connector *nv_connector = nouveau_connector(connector);
+ struct nouveau_conn_atom *armc;
+
+ if (drm_drv_uses_atomic_modeset(connector->dev))
+ armc = nouveau_conn_atom(connector->state);
+ else
+ armc = &nv_connector->properties_state;
/* Init DVI-I specific properties. */
if (connector->connector_type == DRM_MODE_CONNECTOR_DVII)
@@ -749,9 +763,9 @@ static int
nouveau_connector_set_property(struct drm_connector *connector,
struct drm_property *property, uint64_t value)
{
- struct nouveau_conn_atom *asyc = nouveau_conn_atom(connector->state);
struct nouveau_connector *nv_connector = nouveau_connector(connector);
struct nouveau_encoder *nv_encoder = nv_connector->detected_encoder;
+ struct nouveau_conn_atom *asyc = &nv_connector->properties_state;
struct drm_encoder *encoder = to_drm_encoder(nv_encoder);
int ret;
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h
b/drivers/gpu/drm/nouveau/nouveau_connector.h
index de9588420884..de84fb4708c7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -118,6 +118,12 @@ struct nouveau_connector {
#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
struct nouveau_backlight *backlight;
#endif
+ /*
+* Our connector property code expects a nouveau_conn_atom struct
+* even on pre-nv50 where we do not support atomic. This embedded
+* version gets used in the non atomic modeset case.
+*/
+ struct nouveau_conn_atom properties_state;
};
static inline struct nouveau_connector *nouveau_connector(
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/prime: Fix mmap fake offset for drm_gem_object_funcs.mmap
On Wed, Oct 23, 2019 at 05:22:26PM -0500, Rob Herring wrote:
> Commit c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs")
> introduced a GEM object mmap() hook which is expected to subtract the
> fake offset from vm_pgoff.
Long-term it is probably a good idea to just remove the fake offset
handling from drivers. But that'll only work once all drivers switched
away from custom fops->mmap handlers so we can handle the offset -> obj
lookup in the drm core for everybody.
So let's go this way for now.
Acked-by: Gerd Hoffmann
> However, for mmap() on dmabufs, there is not
> a fake offset. To fix this, we need to add the fake offset just like the
> driver->fops->mmap() code path.
>
> Fixes: c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs")
> Cc: Gerd Hoffmann
> Cc: Daniel Vetter
> Signed-off-by: Rob Herring
> ---
> I ran into this while working on converting vgem to shmem helpers and
> the IGT vgem_basic dmabuf-mmap test failed. This fixes shmem, but I
> have checked any other users of the new mmap hook.
> Rob
>
> drivers/gpu/drm/drm_prime.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 0814211b0f3f..5d06690a2e9d 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -713,6 +713,8 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct
> vm_area_struct *vma)
> struct file *fil;
> int ret;
>
> + vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> +
> if (obj->funcs && obj->funcs->mmap) {
> ret = obj->funcs->mmap(obj, vma);
> if (ret)
> @@ -737,8 +739,6 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct
> vm_area_struct *vma)
> if (ret)
> goto out;
>
> - vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> -
> ret = obj->dev->driver->fops->mmap(fil, vma);
>
> drm_vma_node_revoke(&obj->vma_node, priv);
> --
> 2.20.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/nouveau: slowpath for pushbuf ioctl
On Mon, Oct 21, 2019 at 4:50 PM Daniel Vetter wrote:
>
> We can't copy_*_user while holding reservations, that will (soon even
> for nouveau) lead to deadlocks. And it breaks the cross-driver
> contract around dma_resv.
>
> Fix this by adding a slowpath for when we need relocations, and by
> pushing the writeback of the new presumed offsets to the very end.
>
> Aside from "it compiles" entirely untested unfortunately.
>
> Signed-off-by: Daniel Vetter
> Cc: Ilia Mirkin
> Cc: Maarten Lankhorst
> Cc: Ben Skeggs
> Cc: nouv...@lists.freedesktop.org
Ping for review/testing/ack, I'd really like to get this in.
-Daniel
> ---
> drivers/gpu/drm/nouveau/nouveau_gem.c | 57 ++-
> 1 file changed, 38 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c
> b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index 1324c19f4e5c..05ec8edd6a8b 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> @@ -484,12 +484,9 @@ validate_init(struct nouveau_channel *chan, struct
> drm_file *file_priv,
>
> static int
> validate_list(struct nouveau_channel *chan, struct nouveau_cli *cli,
> - struct list_head *list, struct drm_nouveau_gem_pushbuf_bo *pbbo,
> - uint64_t user_pbbo_ptr)
> + struct list_head *list, struct drm_nouveau_gem_pushbuf_bo *pbbo)
> {
> struct nouveau_drm *drm = chan->drm;
> - struct drm_nouveau_gem_pushbuf_bo __user *upbbo =
> - (void __force __user
> *)(uintptr_t)user_pbbo_ptr;
> struct nouveau_bo *nvbo;
> int ret, relocs = 0;
>
> @@ -533,10 +530,6 @@ validate_list(struct nouveau_channel *chan, struct
> nouveau_cli *cli,
> b->presumed.offset = nvbo->bo.offset;
> b->presumed.valid = 0;
> relocs++;
> -
> - if (copy_to_user(&upbbo[nvbo->pbbo_index].presumed,
> -&b->presumed,
> sizeof(b->presumed)))
> - return -EFAULT;
> }
> }
>
> @@ -547,8 +540,8 @@ static int
> nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
> struct drm_file *file_priv,
> struct drm_nouveau_gem_pushbuf_bo *pbbo,
> -uint64_t user_buffers, int nr_buffers,
> -struct validate_op *op, int *apply_relocs)
> +int nr_buffers,
> +struct validate_op *op, bool *apply_relocs)
> {
> struct nouveau_cli *cli = nouveau_cli(file_priv);
> int ret;
> @@ -565,7 +558,7 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
> return ret;
> }
>
> - ret = validate_list(chan, cli, &op->list, pbbo, user_buffers);
> + ret = validate_list(chan, cli, &op->list, pbbo);
> if (unlikely(ret < 0)) {
> if (ret != -ERESTARTSYS)
> NV_PRINTK(err, cli, "validating bo list\n");
> @@ -605,16 +598,12 @@ u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
> static int
> nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
> struct drm_nouveau_gem_pushbuf *req,
> + struct drm_nouveau_gem_pushbuf_reloc *reloc,
> struct drm_nouveau_gem_pushbuf_bo *bo)
> {
> - struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL;
> int ret = 0;
> unsigned i;
>
> - reloc = u_memcpya(req->relocs, req->nr_relocs, sizeof(*reloc));
> - if (IS_ERR(reloc))
> - return PTR_ERR(reloc);
> -
> for (i = 0; i < req->nr_relocs; i++) {
> struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i];
> struct drm_nouveau_gem_pushbuf_bo *b;
> @@ -693,11 +682,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void
> *data,
> struct nouveau_drm *drm = nouveau_drm(dev);
> struct drm_nouveau_gem_pushbuf *req = data;
> struct drm_nouveau_gem_pushbuf_push *push;
> + struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL;
> struct drm_nouveau_gem_pushbuf_bo *bo;
> struct nouveau_channel *chan = NULL;
> struct validate_op op;
> struct nouveau_fence *fence = NULL;
> - int i, j, ret = 0, do_reloc = 0;
> + int i, j, ret = 0;
> + bool do_reloc = false;
>
> if (unlikely(!abi16))
> return -ENOMEM;
> @@ -755,7 +746,8 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void
> *data,
> }
>
> /* Validate buffer list */
> - ret = nouveau_gem_pushbuf_validate(chan, file_priv, bo, req->buffers,
> +revalidate:
> + ret = nouveau_gem_pushbuf_validate(chan, file_priv, bo,
>req->nr_buffers, &op, &do_reloc);
> if (ret) {
>
[Bug 109955] amdgpu [RX Vega 64] system freeze while gaming
https://bugs.freedesktop.org/show_bug.cgi?id=109955 --- Comment #122 from har...@gmx.de --- In my experience, this issue is related to mclk switching and it affects the lowest mclk level only. So you guy's can save a lot of power, if you, insteed of switching to highest gfxlevel or to disable vsync, just disable the lowest mclk level by: echo "manual" > /sys/class/drm/card0/device/power_dpm_force_performance_level echo "1 2 3" > /sys/class/drm/card0/device/pp_dpm_mclk If you are building your kernel locally, look in this thread for a driver code modification that works, without disabling the lowest mclk level (saves a few watt on idle). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109955] amdgpu [RX Vega 64] system freeze while gaming
https://bugs.freedesktop.org/show_bug.cgi?id=109955 --- Comment #123 from har...@gmx.de --- ... i forgot the link to a related thread: https://bugs.freedesktop.org/show_bug.cgi?id=110777 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109887] [Vega10][powerplay] P7 gets reset to max_vddc (1.2V/1.25V) after applying any custom settings via pp_od_clk_voltage and/or pp_table
https://bugs.freedesktop.org/show_bug.cgi?id=109887 --- Comment #15 from Stefan Springer --- The patch Pelle van Gils proposed here works flawlessly for me: https://bugzilla.kernel.org/show_bug.cgi?id=205277 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: use the parent resv for ghost objects v2
Ping?
Am 18.10.19 um 13:58 schrieb Christian König:
This way the TTM is destroyed with the correct dma_resv object
locked and we can even pipeline imported BO evictions.
v2: Limit this to only cases when the parent object uses a separate
reservation object as well. This fixes another OOM problem.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index e030c27f53cf..45e440f80b7b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -512,7 +512,9 @@ static int ttm_buffer_object_transfer(struct
ttm_buffer_object *bo,
kref_init(&fbo->base.kref);
fbo->base.destroy = &ttm_transfered_destroy;
fbo->base.acc_size = 0;
- fbo->base.base.resv = &fbo->base.base._resv;
+ if (bo->base.resv == &bo->base._resv)
+ fbo->base.base.resv = &fbo->base.base._resv;
+
dma_resv_init(fbo->base.base.resv);
ret = dma_resv_trylock(fbo->base.base.resv);
WARN_ON(!ret);
@@ -711,7 +713,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
if (ret)
return ret;
- dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
+ dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
/**
* If we're not moving to fixed memory, the TTM object
@@ -724,7 +726,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
else
bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+ dma_resv_unlock(&ghost_obj->base._resv);
ttm_bo_put(ghost_obj);
}
@@ -767,7 +769,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
if (ret)
return ret;
- dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
+ dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
/**
* If we're not moving to fixed memory, the TTM object
@@ -780,7 +782,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
else
bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+ dma_resv_unlock(&ghost_obj->base._resv);
ttm_bo_put(ghost_obj);
} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
@@ -836,7 +838,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
if (ret)
return ret;
- ret = dma_resv_copy_fences(ghost->base.resv, bo->base.resv);
+ ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv);
/* Last resort, wait for the BO to be idle when we are OOM */
if (ret)
ttm_bo_wait(bo, false, false);
@@ -845,7 +847,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
bo->mem.mem_type = TTM_PL_SYSTEM;
bo->ttm = NULL;
- ttm_bo_unreserve(ghost);
+ dma_resv_unlock(&ghost->base._resv);
ttm_bo_put(ghost);
return 0;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 1/4] drm: Add a new helper drm_color_ctm_s31_32_to_qm_n()
Hi James,
You already have my r-b on the patch, so for posterity: no further
important comments from my side.
On Wednesday, 23 October 2019 07:42:55 BST james qian wang (Arm Technology
China) wrote:
> Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> hardware.
>
> V4: Address Mihai, Daniel and Ilia's review comments.
> V5: Includes the sign bit in the value of m (Qm.n).
> V6: Allows m = 0 according to Mihail's comments.
> V6: Address Mihail's comments.
>
> Signed-off-by: james qian wang (Arm Technology China)
>
> Reviewed-by: Mihail Atanassov
> Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_color_mgmt.c | 36
> include/drm/drm_color_mgmt.h | 2 ++
> 2 files changed, 38 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
> b/drivers/gpu/drm/drm_color_mgmt.c
> index 4ce5c6d8de99..f5fba5802a07 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -132,6 +132,42 @@ uint32_t drm_color_lut_extract(uint32_t user_input,
> uint32_t bit_precision)
> }
> EXPORT_SYMBOL(drm_color_lut_extract);
>
> +/**
> + * drm_color_ctm_s31_32_to_qm_n
> + *
> + * @user_input: input value
> + * @m: number of integer bits, only support m <= 32, include the sign-bit
> + * @n: number of fractional bits, only support n <= 32
> + *
> + * Convert and clamp S31.32 sign-magnitude to Qm.n (signed 2's complement).
> + * The sign-bit BIT(m+n) and above are 0 for positive value and 1 for
> negative.
[really pedantic] m+n-1 :)
> + * the range of value is [-2^(m-1), 2^(m-1) - 2^-n]
> + *
> + * For example
> + * A Q3.12 format number:
> + * - required bit: 3 + 12 = 15bits
> + * - range: [-2^2, 2^2 - 2^−15]
> + *
> + * NOTE: the m can be zero if all bit_precision are used to present
> fractional
> + * bits like Q0.32
> + */
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> + uint32_t m, uint32_t n)
> +{
> + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> + bool negative = !!(user_input & BIT_ULL(63));
> + s64 val;
> +
> + WARN_ON(m > 32 || n > 32);
> +
> +
> + val = clamp_val(mag, 0, negative ?
> + BIT_ULL(n + m - 1) : BIT_ULL(n + m - 1) - 1);
> +
> + return negative ? -val : val;
> +}
> +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> +
> /**
> * drm_crtc_enable_color_mgmt - enable color management properties
> * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d92ab7..60fea5501886 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -30,6 +30,8 @@ struct drm_crtc;
> struct drm_plane;
>
> uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> + uint32_t m, uint32_t n);
>
> void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> uint degamma_lut_size,
> --
> 2.20.1
>
>
--
Mihail
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/dp: Add function to parse EDID descriptors for adaptive sync limits
On Wed, Oct 23, 2019 at 05:00:41PM -0700, Manasi Navare wrote:
> Adaptive Sync is a VESA feature so add a DRM core helper to parse
> the EDID's detailed descritors to obtain the adaptive sync monitor range.
> Store this info as part fo drm_display_info so it can be used
> across all drivers.
> This part of the code is stripped out of amdgpu's function
> amdgpu_dm_update_freesync_caps() to make it generic and be used
> across all DRM drivers
>
> Cc: Ville Syrjälä
> Cc: Harry Wentland
> Cc: Clinton A Taylor
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/drm_edid.c | 49 +
> include/drm/drm_connector.h | 25 +++
> include/drm/drm_edid.h | 2 ++
> 3 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 474ac04d5600..97dd1200773e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4707,6 +4707,52 @@ static void drm_parse_cea_ext(struct drm_connector
> *connector,
> }
> }
>
> +void drm_get_adaptive_sync_limits(struct drm_connector *connector,
> + const struct edid *edid)
> +{
> + struct drm_display_info *info = &connector->display_info;
> + const struct detailed_timing *timing;
> + const struct detailed_non_pixel *data;
> + const struct detailed_data_monitor_range *range;
> + int i;
This can be unsigned int.
> +
> + /*
> + * Restrict Adaptive Sync only for dp and edp
> + */
> + if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
> + connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> + return;
> +
> + if (edid->version <= 1 && !(edid->version == 1 && edid->revision > 1))
> + return;
> +
> + for (i = 0; i < 4; i++) {
> + timing = &edid->detailed_timings[i];
> + data= &timing->data.other_data;
> + range = &data->data.range;
> + /*
> + * Check if monitor has continuous frequency mode
> + */
> + if (data->type != EDID_DETAIL_MONITOR_RANGE)
> + continue;
> + /*
> + * Check for flag range limits only. If flag == 1 then
> + * no additional timing information provided.
> + * Default GTF, GTF Secondary curve and CVT are not
> + * supported
> + */
> + if (range->flags != 1)
> + continue;
> +
> + info->adaptive_sync.min_vfreq = range->min_vfreq;
> + info->adaptive_sync.max_vfreq = range->max_vfreq;
> + info->adaptive_sync.pixel_clock_mhz =
> + range->pixel_clock_mhz * 10;
> + break;
> + }
> +}
> +EXPORT_SYMBOL(drm_get_adaptive_sync_limits);
> +
> /* A connector has no EDID information, so we've got no EDID to compute
> quirks from. Reset
> * all of the values which would have been set from EDID
> */
> @@ -4728,6 +4774,7 @@ drm_reset_display_info(struct drm_connector *connector)
> memset(&info->hdmi, 0, sizeof(info->hdmi));
>
> info->non_desktop = 0;
> + memset(&info->adaptive_sync, 0, sizeof(info->adaptive_sync));
> }
>
> u32 drm_add_display_info(struct drm_connector *connector, const struct edid
> *edid)
> @@ -4743,6 +4790,8 @@ u32 drm_add_display_info(struct drm_connector
> *connector, const struct edid *edi
>
> info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
>
> + drm_get_adaptive_sync_limits(connector, edid);
> +
> DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
>
> if (edid->revision < 3)
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 5f8c3389d46f..a27a84270d8d 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -254,6 +254,26 @@ enum drm_panel_orientation {
> DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> };
>
> +/**
> + * struct drm_adaptive_sync_info - Panel's Adaptive Sync capabilities for
> + * &drm_display_info
> + *
> + * This struct is used to store a Panel's Adaptive Sync capabilities
> + * as parsed from EDID's detailed monitor range descriptor block.
> + *
> + * @min_vfreq: This is the min supported refresh rate in Hz from
> + * EDID's detailed monitor range.
> + * @max_vfreq: This is the max supported refresh rate in Hz from
> + * EDID's detailed monitor range
> + * @pixel_clock_mhz: This is the dotclock in MHz from
> + * EDID's detailed monitor range
> + */
> +struct drm_adaptive_sync_info {
> + int min_vfreq;
> + int max_vfreq;
> + int pixel_clock_mhz;
Any reason why these can't be unsigned? Also, does it perhaps make sense
to store the pixel clock as kHz like we do everywhere else?
Thierry
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.fr
Re: [Intel-gfx] [PATCH 2/2] drm/todo: Add entry to remove load/unload hooks
On Wed, Oct 23, 2019 at 04:49:53PM +0200, Daniel Vetter wrote: > They're midlayer, broken, and because of the old gunk, we can't fix > them. For examples see the various checks in drm_mode_object.c against > dev->registered, which cannot be enforced if the driver still uses the > load hook. > > Unfortunately our biggest driver still uses load/unload, so this would > be really great to get fixed. > > Cc: Alex Deucher > Cc: Harry Wentland > Signed-off-by: Daniel Vetter > --- > Documentation/gpu/todo.rst | 17 + > 1 file changed, 17 insertions(+) Reminds me that I need to still do that for Tegra: Reviewed-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] drm/property: Enforce more lifetime rules
On Wed, Oct 23, 2019 at 04:49:52PM +0200, Daniel Vetter wrote:
> Properties can't be attached after registering, userspace would get
> confused (no one bothers to reprobe really).
>
> - Add kerneldoc
> - Enforce this with some checks. This needs a somewhat ugly check
> since connectors can be added later on, but we still need to attach
> all properties before they go public.
>
> Note that we already enforce that properties themselves are created
> before the entire device is registered.
>
> Cc: Jani Nikula
> Cc: Rajat Jain
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_mode_object.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_object.c
> b/drivers/gpu/drm/drm_mode_object.c
> index 6a23e36ed4fe..35c2719407a8 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -224,12 +224,26 @@ EXPORT_SYMBOL(drm_mode_object_get);
> * This attaches the given property to the modeset object with the given
> initial
> * value. Currently this function cannot fail since the properties are
> stored in
> * a statically sized array.
> + *
> + * Note that all properties must be attached before the object itself is
> + * registered and accessible from userspace.
> */
> void drm_object_attach_property(struct drm_mode_object *obj,
> struct drm_property *property,
> uint64_t init_val)
> {
> int count = obj->properties->count;
> + struct drm_device *dev = property->dev;
> +
> +
> + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
> + struct drm_connector *connector = obj_to_connector(obj);
> +
> + WARN_ON(!dev->driver->load &&
> + connector->registration_state ==
> DRM_CONNECTOR_REGISTERED);
> + } else {
> + WARN_ON(!dev->driver->load && dev->registered);
> + }
I'm not sure I understand why dev->driver->load needs to be a special
case. Don't the same rules apply for those drivers as well?
Thierry
>
> if (count == DRM_OBJECT_MAX_PROPERTY) {
> WARN(1, "Failed to attach object property (type: 0x%x). Please "
> --
> 2.23.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] drm/property: Enforce more lifetime rules
On Thu, Oct 24, 2019 at 12:40:55PM +0200, Thierry Reding wrote:
> On Wed, Oct 23, 2019 at 04:49:52PM +0200, Daniel Vetter wrote:
> > Properties can't be attached after registering, userspace would get
> > confused (no one bothers to reprobe really).
> >
> > - Add kerneldoc
> > - Enforce this with some checks. This needs a somewhat ugly check
> > since connectors can be added later on, but we still need to attach
> > all properties before they go public.
> >
> > Note that we already enforce that properties themselves are created
> > before the entire device is registered.
> >
> > Cc: Jani Nikula
> > Cc: Rajat Jain
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_mode_object.c | 14 ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_mode_object.c
> > b/drivers/gpu/drm/drm_mode_object.c
> > index 6a23e36ed4fe..35c2719407a8 100644
> > --- a/drivers/gpu/drm/drm_mode_object.c
> > +++ b/drivers/gpu/drm/drm_mode_object.c
> > @@ -224,12 +224,26 @@ EXPORT_SYMBOL(drm_mode_object_get);
> > * This attaches the given property to the modeset object with the given
> > initial
> > * value. Currently this function cannot fail since the properties are
> > stored in
> > * a statically sized array.
> > + *
> > + * Note that all properties must be attached before the object itself is
> > + * registered and accessible from userspace.
> > */
> > void drm_object_attach_property(struct drm_mode_object *obj,
> > struct drm_property *property,
> > uint64_t init_val)
> > {
> > int count = obj->properties->count;
> > + struct drm_device *dev = property->dev;
> > +
> > +
> > + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
> > + struct drm_connector *connector = obj_to_connector(obj);
> > +
> > + WARN_ON(!dev->driver->load &&
> > + connector->registration_state ==
> > DRM_CONNECTOR_REGISTERED);
> > + } else {
> > + WARN_ON(!dev->driver->load && dev->registered);
> > + }
>
> I'm not sure I understand why dev->driver->load needs to be a special
> case. Don't the same rules apply for those drivers as well?
Nevermind, I just noticed that drm_dev_register() sets dev->registered
to true before calling the driver's ->load() implementation, so makes
sense:
Reviewed-by: Thierry Reding
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/v3d: fix double free of bin
From: Colin Ian King
Two different fixes have addressed the same memory leak of bin and
this now causes a double free of bin. While the individual memory
leak fixes are fine, both fixes together are problematic.
Addresses-Coverity: ("Double free")
Fixes: 29cd13cfd762 ("drm/v3d: Fix memory leak in v3d_submit_cl_ioctl")
Fixes: 0d352a3a8a1f (" rm/v3d: don't leak bin job if v3d_job_init fails.")
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/v3d/v3d_gem.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 549dde83408b..37515e47b47e 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -568,7 +568,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
ret = v3d_job_init(v3d, file_priv, &bin->base,
v3d_job_free, args->in_sync_bcl);
if (ret) {
- kfree(bin);
v3d_job_put(&render->base);
kfree(bin);
return ret;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: use the parent resv for ghost objects v2
On 2019/10/24 下午6:25, Christian König wrote:
> Ping?
>
> Am 18.10.19 um 13:58 schrieb Christian König:
>> This way the TTM is destroyed with the correct dma_resv object
>> locked and we can even pipeline imported BO evictions.
>>
>> v2: Limit this to only cases when the parent object uses a separate
>> reservation object as well. This fixes another OOM problem.
>>
>> Signed-off-by: Christian König
>> ---
>> drivers/gpu/drm/ttm/ttm_bo_util.c | 16 +---
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index e030c27f53cf..45e440f80b7b 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -512,7 +512,9 @@ static int ttm_buffer_object_transfer(struct
>> ttm_buffer_object *bo,
>> kref_init(&fbo->base.kref);
>> fbo->base.destroy = &ttm_transfered_destroy;
>> fbo->base.acc_size = 0;
>> - fbo->base.base.resv = &fbo->base.base._resv;
>> + if (bo->base.resv == &bo->base._resv)
>> + fbo->base.base.resv = &fbo->base.base._resv;
>> +
>> dma_resv_init(fbo->base.base.resv);
Doesn't this lead to issue if you force to init parent resv? Otherwise
how to deal with if parent->resv is locking?
>> ret = dma_resv_trylock(fbo->base.base.resv);
>> WARN_ON(!ret);
>> @@ -711,7 +713,7 @@ int ttm_bo_move_accel_cleanup(struct
>> ttm_buffer_object *bo,
>> if (ret)
>> return ret;
>> - dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
>> + dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
>> /**
>> * If we're not moving to fixed memory, the TTM object
>> @@ -724,7 +726,7 @@ int ttm_bo_move_accel_cleanup(struct
>> ttm_buffer_object *bo,
>> else
>> bo->ttm = NULL;
>> - ttm_bo_unreserve(ghost_obj);
>> + dma_resv_unlock(&ghost_obj->base._resv);
fbo->base.base.resv?
-David
>> ttm_bo_put(ghost_obj);
>> }
>> @@ -767,7 +769,7 @@ int ttm_bo_pipeline_move(struct
>> ttm_buffer_object *bo,
>> if (ret)
>> return ret;
>> - dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
>> + dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
>> /**
>> * If we're not moving to fixed memory, the TTM object
>> @@ -780,7 +782,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object
>> *bo,
>> else
>> bo->ttm = NULL;
>> - ttm_bo_unreserve(ghost_obj);
>> + dma_resv_unlock(&ghost_obj->base._resv);
>> ttm_bo_put(ghost_obj);
>> } else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> @@ -836,7 +838,7 @@ int ttm_bo_pipeline_gutting(struct
>> ttm_buffer_object *bo)
>> if (ret)
>> return ret;
>> - ret = dma_resv_copy_fences(ghost->base.resv, bo->base.resv);
>> + ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv);
>> /* Last resort, wait for the BO to be idle when we are OOM */
>> if (ret)
>> ttm_bo_wait(bo, false, false);
>> @@ -845,7 +847,7 @@ int ttm_bo_pipeline_gutting(struct
>> ttm_buffer_object *bo)
>> bo->mem.mem_type = TTM_PL_SYSTEM;
>> bo->ttm = NULL;
>> - ttm_bo_unreserve(ghost);
>> + dma_resv_unlock(&ghost->base._resv);
>> ttm_bo_put(ghost);
>> return 0;
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: use the parent resv for ghost objects v2
Am 24.10.19 um 12:51 schrieb Zhou, David(ChunMing):
On 2019/10/24 下午6:25, Christian König wrote:
Ping?
Am 18.10.19 um 13:58 schrieb Christian König:
This way the TTM is destroyed with the correct dma_resv object
locked and we can even pipeline imported BO evictions.
v2: Limit this to only cases when the parent object uses a separate
reservation object as well. This fixes another OOM problem.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index e030c27f53cf..45e440f80b7b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -512,7 +512,9 @@ static int ttm_buffer_object_transfer(struct
ttm_buffer_object *bo,
kref_init(&fbo->base.kref);
fbo->base.destroy = &ttm_transfered_destroy;
fbo->base.acc_size = 0;
- fbo->base.base.resv = &fbo->base.base._resv;
+ if (bo->base.resv == &bo->base._resv)
+ fbo->base.base.resv = &fbo->base.base._resv;
+
dma_resv_init(fbo->base.base.resv);
Doesn't this lead to issue if you force to init parent resv? Otherwise
how to deal with if parent->resv is locking?
Ups, good point. That is indeed a really bad typo added during the
rebase. Going to fix that.
Thanks,
Christian.
ret = dma_resv_trylock(fbo->base.base.resv);
WARN_ON(!ret);
@@ -711,7 +713,7 @@ int ttm_bo_move_accel_cleanup(struct
ttm_buffer_object *bo,
if (ret)
return ret;
- dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
+ dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
/**
* If we're not moving to fixed memory, the TTM object
@@ -724,7 +726,7 @@ int ttm_bo_move_accel_cleanup(struct
ttm_buffer_object *bo,
else
bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+ dma_resv_unlock(&ghost_obj->base._resv);
fbo->base.base.resv?
-David
ttm_bo_put(ghost_obj);
}
@@ -767,7 +769,7 @@ int ttm_bo_pipeline_move(struct
ttm_buffer_object *bo,
if (ret)
return ret;
- dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
+ dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
/**
* If we're not moving to fixed memory, the TTM object
@@ -780,7 +782,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object
*bo,
else
bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+ dma_resv_unlock(&ghost_obj->base._resv);
ttm_bo_put(ghost_obj);
} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
@@ -836,7 +838,7 @@ int ttm_bo_pipeline_gutting(struct
ttm_buffer_object *bo)
if (ret)
return ret;
- ret = dma_resv_copy_fences(ghost->base.resv, bo->base.resv);
+ ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv);
/* Last resort, wait for the BO to be idle when we are OOM */
if (ret)
ttm_bo_wait(bo, false, false);
@@ -845,7 +847,7 @@ int ttm_bo_pipeline_gutting(struct
ttm_buffer_object *bo)
bo->mem.mem_type = TTM_PL_SYSTEM;
bo->ttm = NULL;
- ttm_bo_unreserve(ghost);
+ dma_resv_unlock(&ghost->base._resv);
ttm_bo_put(ghost);
return 0;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205277] [amd powerplay] vega10: soc voltage for power state 7 is not changed by overdrive.
https://bugzilla.kernel.org/show_bug.cgi?id=205277 har...@gmx.de changed: What|Removed |Added CC||har...@gmx.de --- Comment #3 from har...@gmx.de --- Did you debug this issue? I think the problem could be outside this code. I would outcomment the if-statement following for-loop in your proposed patch, because otherwise 'i' points outside the array boundarys here. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: use the parent resv for ghost objects v3
This way the TTM is destroyed with the correct dma_resv object
locked and we can even pipeline imported BO evictions.
v2: Limit this to only cases when the parent object uses a separate
reservation object as well. This fixes another OOM problem.
v3: fix init and try_lock on the wrong object
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 73a1b0186029..f7b57ca1a95b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -516,9 +516,11 @@ static int ttm_buffer_object_transfer(struct
ttm_buffer_object *bo,
kref_init(&fbo->base.kref);
fbo->base.destroy = &ttm_transfered_destroy;
fbo->base.acc_size = 0;
- fbo->base.base.resv = &fbo->base.base._resv;
- dma_resv_init(fbo->base.base.resv);
- ret = dma_resv_trylock(fbo->base.base.resv);
+ if (bo->base.resv == &bo->base._resv)
+ fbo->base.base.resv = &fbo->base.base._resv;
+
+ dma_resv_init(&fbo->base.base._resv);
+ ret = dma_resv_trylock(&fbo->base.base._resv);
WARN_ON(!ret);
*new_obj = &fbo->base;
@@ -715,7 +717,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
if (ret)
return ret;
- dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
+ dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
/**
* If we're not moving to fixed memory, the TTM object
@@ -728,7 +730,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
else
bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+ dma_resv_unlock(&ghost_obj->base._resv);
ttm_bo_put(ghost_obj);
}
@@ -771,7 +773,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
if (ret)
return ret;
- dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
+ dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
/**
* If we're not moving to fixed memory, the TTM object
@@ -784,7 +786,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
else
bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+ dma_resv_unlock(&ghost_obj->base._resv);
ttm_bo_put(ghost_obj);
} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
@@ -840,7 +842,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
if (ret)
return ret;
- ret = dma_resv_copy_fences(ghost->base.resv, bo->base.resv);
+ ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv);
/* Last resort, wait for the BO to be idle when we are OOM */
if (ret)
ttm_bo_wait(bo, false, false);
@@ -849,7 +851,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
bo->mem.mem_type = TTM_PL_SYSTEM;
bo->ttm = NULL;
- ttm_bo_unreserve(ghost);
+ dma_resv_unlock(&ghost->base._resv);
ttm_bo_put(ghost);
return 0;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Add support for integrated privacy screens
On Tue, Oct 22, 2019 at 05:12:06PM -0700, Rajat Jain wrote:
> Certain laptops now come with panels that have integrated privacy
> screens on them. This patch adds support for such panels by adding
> a privacy-screen property to the drm_connector for the panel, that
> the userspace can then use to control and check the status. The idea
> was discussed here:
>
> https://lkml.org/lkml/2019/10/1/786
>
> ACPI methods are used to identify, query and control privacy screen:
>
> * Identifying an ACPI object corresponding to the panel: The patch
> follows ACPI Spec 6.3 (available at
> https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf).
> Pages 1119 - 1123 describe what I believe, is a standard way of
> identifying / addressing "display panels" in the ACPI tables, thus
> allowing kernel to attach ACPI nodes to the panel. IMHO, this ability
> to identify and attach ACPI nodes to drm connectors may be useful for
> reasons other privacy-screens, in future.
>
> * Identifying the presence of privacy screen, and controlling it, is done
> via ACPI _DSM methods.
>
> Currently, this is done only for the Intel display ports. But in future,
> this can be done for any other ports if the hardware becomes available
> (e.g. external monitors supporting integrated privacy screens?).
>
> Also, this code can be extended in future to support non-ACPI methods
> (e.g. using a kernel GPIO driver to toggle a gpio that controls the
> privacy-screen).
>
> Signed-off-by: Rajat Jain
> ---
> drivers/gpu/drm/Makefile| 1 +
> drivers/gpu/drm/drm_atomic_uapi.c | 5 +
> drivers/gpu/drm/drm_connector.c | 38 +
> drivers/gpu/drm/drm_privacy_screen.c| 176
> drivers/gpu/drm/i915/display/intel_dp.c | 3 +
> include/drm/drm_connector.h | 18 +++
> include/drm/drm_mode_config.h | 7 +
> include/drm/drm_privacy_screen.h| 33 +
> 8 files changed, 281 insertions(+)
> create mode 100644 drivers/gpu/drm/drm_privacy_screen.c
> create mode 100644 include/drm/drm_privacy_screen.h
I like this much better than the prior proposal to use sysfs. However
the support currently looks a bit tangled. I realize that we only have a
single implementation for this in hardware right now, so there's no use
in over-engineering things, but I think we can do a better job from the
start without getting into too many abstractions. See below.
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 82ff826b33cc..e1fc33d69bb7 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -19,6 +19,7 @@ drm-y := drm_auth.o drm_cache.o \
> drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
> drm_client_modeset.o drm_atomic_uapi.o drm_hdcp.o
>
> +drm-$(CONFIG_ACPI) += drm_privacy_screen.o
> drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o
> drm_dma.o drm_scatter.o drm_lock.o
> drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> drm-$(CONFIG_DRM_VM) += drm_vm.o
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 7a26bfb5329c..44131165e4ea 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -766,6 +767,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> fence_ptr);
> } else if (property == connector->max_bpc_property) {
> state->max_requested_bpc = val;
> + } else if (property == config->privacy_screen_property) {
> + drm_privacy_screen_set_val(connector, val);
This doesn't look right. Shouldn't you store the value in the connector
state and then leave it up to the connector driver to set it
appropriately? I think that also has the advantage of untangling this
support a little.
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -842,6 +845,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = 0;
> } else if (property == connector->max_bpc_property) {
> *val = state->max_requested_bpc;
> + } else if (property == config->privacy_screen_property) {
> + *val = drm_privacy_screen_get_val(connector);
Similarly, I think this can just return the atomic state's value for
this.
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..a31e0382132b 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/
Re: [PATCH v3 00/21] drm: Add support for bus-format negotiation
hi Boris, On 23/10/2019 17:44, Boris Brezillon wrote: > This patch series aims at adding support for runtime bus-format > negotiation between all elements of the > 'encoder -> bridges -> connector/display' section of the pipeline. > > In order to support that, we need drm bridges to fully take part in the > atomic state validation process, which requires adding a > drm_bridge_state and a new drm_bridge_funcs.atomic_check() hook. > Once those basic building blocks are in place, we can add new hooks to > allow bus format negotiation (those are called just before > ->atomic_check()). The bus format selection is done at runtime by > testing all possible combinations across the whole bridge chain until > one is reported to work. > > Major changes since v2: > * Get rid of the dummy bridge embedded in drm_encoder and let encoder > drivers provide their own bridge element > * Clarify APIs and improve the doc > * Propagate bus flags by default Seems you forgot my reviewed-bys on patches 5, 8, 11 & 13 > > Major changes since the RFC: > > * Add a dummy bridge to the drm_encoder object so that vc4 and exynos > DSI drivers can implement the pre_enable/post_disable hooks instead > of manually setting encoder->bridge to NULL to control the > enable/disable sequence. This change is also a first step towards > drm_bridge/drm_encoder unification. New encoder drivers should > stop implementing drm_encoder_helper_funcs and switch to > drm_bridge_funcs. Existing drivers can be converted progressively > (already have a branch where I started converting some of them [1]) > * rework the bus format negotiation to give more control to bridge > drivers in the selection process (driver can select at runtime which > input bus format they support for a specific output bus format based > on any information available in the connector, crtc and bridge state. > > A more detailed changelog is provided in each patch. > > This patch series is also available here [2]. Will test ASAP. > > Thanks, > > Boris > > [1]https://github.com/bbrezillon/linux-0day/commits/drm-encoder-bridge > [2]https://github.com/bbrezillon/linux-0day/commits/drm-bridge-busfmt-v3 > > *** BLURB HERE *** Blurp Neil > > Boris Brezillon (21): > drm/vc4: Declare the DSI encoder as a bridge element > drm/exynos: Don't reset bridge->next > drm/exynos: Declare the DSI encoder as a bridge element > drm/bridge: Rename bridge helpers targeting a bridge chain > drm/bridge: Introduce drm_bridge_chain_get_next_bridge() > drm: Stop accessing encoder->bridge directly > drm/bridge: Make the bridge chain a double-linked list > drm/bridge: Add the drm_for_each_bridge_in_chain() helper > drm/bridge: Add a drm_bridge_state object > drm/bridge: Clarify the atomic enable/disable hooks semantics > drm/bridge: Patch atomic hooks to take a drm_bridge_state > drm/bridge: Add an ->atomic_check() hook > drm/bridge: Add the drm_bridge_chain_get_prev_bridge() helper > drm/bridge: Add the necessary bits to support bus format negotiation > drm/imx: pd: Use bus format/flags provided by the bridge when > available > drm/bridge: lvds-encoder: Implement basic bus format negotiation > dt-bindings: display: bridge: lvds-transmitter: Add new props > drm/bridge: panel: Propage bus format/flags > drm/panel: simple: Add support for Toshiba LTA089AC29000 panel > dt-bindings: display: panel: Add the LTA089AC29000 variant > ARM: dts: imx: imx51-zii-rdu1: Fix the display pipeline definition > > .../display/bridge/lvds-transmitter.txt | 13 + > .../display/panel/toshiba,lt089ac29000.txt| 5 +- > arch/arm/boot/dts/imx51-zii-rdu1.dts | 24 +- > .../drm/bridge/analogix/analogix_dp_core.c| 12 +- > drivers/gpu/drm/bridge/lvds-encoder.c | 72 ++ > drivers/gpu/drm/bridge/panel.c| 1 + > drivers/gpu/drm/drm_atomic.c | 39 + > drivers/gpu/drm/drm_atomic_helper.c | 54 +- > drivers/gpu/drm/drm_bridge.c | 800 +++--- > drivers/gpu/drm/drm_encoder.c | 15 +- > drivers/gpu/drm/drm_probe_helper.c| 4 +- > drivers/gpu/drm/exynos/exynos_dp.c| 1 - > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 90 +- > drivers/gpu/drm/imx/parallel-display.c| 174 +++- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 8 +- > drivers/gpu/drm/msm/edp/edp_bridge.c | 10 +- > drivers/gpu/drm/omapdrm/omap_drv.c| 4 +- > drivers/gpu/drm/omapdrm/omap_encoder.c| 3 +- > drivers/gpu/drm/panel/panel-simple.c | 36 + > drivers/gpu/drm/rcar-du/rcar_du_crtc.c| 11 +- > drivers/gpu/drm/vc4/vc4_dsi.c | 90 +- > include/drm/drm_atomic.h | 3 + > include/drm/drm_bridge.h | 396 - > include/drm/drm_encoder.h | 9 +- > 24 files changed, 1588 insertions(+), 286 deletions(
Re: [Intel-gfx] [PATCH] drm/dp: Add function to parse EDID descriptors for adaptive sync limits
On Thu, Oct 24, 2019 at 12:31:06PM +0200, Thierry Reding wrote:
> On Wed, Oct 23, 2019 at 05:00:41PM -0700, Manasi Navare wrote:
> > Adaptive Sync is a VESA feature so add a DRM core helper to parse
> > the EDID's detailed descritors to obtain the adaptive sync monitor range.
> > Store this info as part fo drm_display_info so it can be used
> > across all drivers.
> > This part of the code is stripped out of amdgpu's function
> > amdgpu_dm_update_freesync_caps() to make it generic and be used
> > across all DRM drivers
> >
> > Cc: Ville Syrjälä
> > Cc: Harry Wentland
> > Cc: Clinton A Taylor
> > Signed-off-by: Manasi Navare
> > ---
> > drivers/gpu/drm/drm_edid.c | 49 +
> > include/drm/drm_connector.h | 25 +++
> > include/drm/drm_edid.h | 2 ++
> > 3 files changed, 76 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 474ac04d5600..97dd1200773e 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -4707,6 +4707,52 @@ static void drm_parse_cea_ext(struct drm_connector
> > *connector,
> > }
> > }
> >
> > +void drm_get_adaptive_sync_limits(struct drm_connector *connector,
> > + const struct edid *edid)
> > +{
> > + struct drm_display_info *info = &connector->display_info;
> > + const struct detailed_timing *timing;
> > + const struct detailed_non_pixel *data;
> > + const struct detailed_data_monitor_range *range;
> > + int i;
>
> This can be unsigned int.
Please no. A loop iterator called 'i' should always be a normal signed int.
>
> > +
> > + /*
> > +* Restrict Adaptive Sync only for dp and edp
> > +*/
> > + if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
> > + connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> > + return;
> > +
> > + if (edid->version <= 1 && !(edid->version == 1 && edid->revision > 1))
> > + return;
> > +
> > + for (i = 0; i < 4; i++) {
> > + timing = &edid->detailed_timings[i];
> > + data= &timing->data.other_data;
> > + range = &data->data.range;
> > + /*
> > +* Check if monitor has continuous frequency mode
> > +*/
> > + if (data->type != EDID_DETAIL_MONITOR_RANGE)
> > + continue;
> > + /*
> > +* Check for flag range limits only. If flag == 1 then
> > +* no additional timing information provided.
> > +* Default GTF, GTF Secondary curve and CVT are not
> > +* supported
> > +*/
> > + if (range->flags != 1)
> > + continue;
> > +
> > + info->adaptive_sync.min_vfreq = range->min_vfreq;
> > + info->adaptive_sync.max_vfreq = range->max_vfreq;
> > + info->adaptive_sync.pixel_clock_mhz =
> > + range->pixel_clock_mhz * 10;
> > + break;
> > + }
> > +}
> > +EXPORT_SYMBOL(drm_get_adaptive_sync_limits);
> > +
> > /* A connector has no EDID information, so we've got no EDID to compute
> > quirks from. Reset
> > * all of the values which would have been set from EDID
> > */
> > @@ -4728,6 +4774,7 @@ drm_reset_display_info(struct drm_connector
> > *connector)
> > memset(&info->hdmi, 0, sizeof(info->hdmi));
> >
> > info->non_desktop = 0;
> > + memset(&info->adaptive_sync, 0, sizeof(info->adaptive_sync));
> > }
> >
> > u32 drm_add_display_info(struct drm_connector *connector, const struct
> > edid *edid)
> > @@ -4743,6 +4790,8 @@ u32 drm_add_display_info(struct drm_connector
> > *connector, const struct edid *edi
> >
> > info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
> >
> > + drm_get_adaptive_sync_limits(connector, edid);
> > +
> > DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
> >
> > if (edid->revision < 3)
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 5f8c3389d46f..a27a84270d8d 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -254,6 +254,26 @@ enum drm_panel_orientation {
> > DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> > };
> >
> > +/**
> > + * struct drm_adaptive_sync_info - Panel's Adaptive Sync capabilities for
> > + * &drm_display_info
> > + *
> > + * This struct is used to store a Panel's Adaptive Sync capabilities
> > + * as parsed from EDID's detailed monitor range descriptor block.
> > + *
> > + * @min_vfreq: This is the min supported refresh rate in Hz from
> > + * EDID's detailed monitor range.
> > + * @max_vfreq: This is the max supported refresh rate in Hz from
> > + * EDID's detailed monitor range
> > + * @pixel_clock_mhz: This is the dotclock in MHz from
> > + * EDID's detailed monitor range
> > + */
> > +struct drm_adaptive_sync_info {
> > + int min_vfreq;
> > + int max_vfreq;
> >
[PATCH 1/3 v4] drm/panel: Add generic DSI panel YAML bindings
This adds a starting point for processing and defining generic
bindings used by DSI panels. We just define one single bool
property to force the panel into video mode for now.
Cc: devicet...@vger.kernel.org
Suggested-by: Rob Herring
Signed-off-by: Linus Walleij
---
ChangeLog v3->v4:
- Rename into display/dsi-controller.yaml
- Require a virtual channel number for the DSI panel, as
DSI have this 2-bit virtual address field.
- Bring in some but not all properties from the existing MIPI
DSI bindings. This schema can be used with simpler panels but
not complex panels with multiple virtual channels for the
moment. Let's handle it when we get there.
- Add an example.
ChangeLog v2->v3:
- Make a more complete DSI panel binding including the controller
and its address-cells and size-cells and a pattern for the panel
nodes. The panel is one per DSI master, the reg property is
compulsory but should always be 0 (as far as I can tell) as
only one panel can be connected. The bus doesn't really have
any addresses for the panel, the address/reg notation seems
to be cargo-culted from the port graphs and is not necessary
to parse some device trees, it is used to tell whether the
node is a panel or not rather than any addressing.
- I have no idea how many displays you can daisychain on a single
DSI master, I just guess 15 will be enough. The MIPI-specs
are memberwalled. Someone who knows can tell perhaps?
ChangeLog v1->v2:
- New patch after feedback.
---
.../bindings/display/dsi-controller.yaml | 88 +++
1 file changed, 88 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/dsi-controller.yaml
diff --git a/Documentation/devicetree/bindings/display/dsi-controller.yaml
b/Documentation/devicetree/bindings/display/dsi-controller.yaml
new file mode 100644
index ..2a6d872a40c5
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/dsi-controller.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/panel-dsi-common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Properties for DSI Display Panels
+
+maintainers:
+ - Linus Walleij
+
+description: |
+ This document defines device tree properties common to DSI, Display
+ Serial Interface panels. It doesn't constitute a device tree binding
+ specification by itself but is meant to be referenced by device tree
+ bindings.
+
+ When referenced from panel device tree bindings the properties defined in
+ this document are defined as follows. The panel device tree bindings are
+ responsible for defining whether each property is required or optional.
+
+ Notice: this binding concerns DSI panels connected directly to a master
+ without any intermediate port graph to the panel. Each DSI master
+ can control exactly one panel. They should all just have a node "panel"
+ for their panel with their reg-property set to 0.
+
+properties:
+ $nodename:
+pattern: "^dsi-controller(@.*)?$"
+
+ "#address-cells":
+const: 1
+
+ "#size-cells":
+const: 0
+
+patternProperties:
+ "^panel@[0-3]$":
+type: object
+
+properties:
+ reg:
+minimum: 0
+maximum: 3
+description:
+ The virtual channel number of a DSI peripheral. Must be in the range
+ from 0 to 3, as DSI uses a 2-bit addressing scheme. Some DSI
+ peripherals respond to more than a single virtual channel. In that
+ case the reg property can take multiple entries, one for each virtual
+ channel that the peripheral responds to.
+
+ clock-master:
+type: boolean
+description:
+ Should be enabled if the host is being used in conjunction with
+ another DSI host to drive the same peripheral. Hardware supporting
+ such a configuration generally requires the data on both the busses
+ to be driven by the same clock. Only the DSI host instance
+ controlling this clock should contain this property.
+
+ enforce-video-mode:
+type: boolean
+description:
+ The best option is usually to run a panel in command mode, as this
+ gives better control over the panel hardware. However for different
+ reasons like broken hardware, missing features or testing, it may be
+ useful to be able to force a command mode-capable panel into video
+ mode.
+
+required:
+ - reg
+
+examples:
+ - |
+dsi-controller@55aa55aa {
+ compatible = "acme,foo";
+ reg = <0x55aa55aa>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ panel@0 {
+compatible = "acme,bar";
+reg = <0>;
+vddi-supply = <&foo>;
+reset-gpios = <&foo_gpio 0 GPIO_ACTIVE_LOW>;
+ };
+};
+
+...
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedeskto
[PATCH 2/3 v4] drm/panel: Add DT bindings for Sony ACX424AKP
This adds device tree bindings for the Sony ACX424AKP panel.
Let's use YAML.
Cc: devicet...@vger.kernel.org
Signed-off-by: Linus Walleij
---
ChangeLog v3->v4:
- Adjust to adjusted DSI bindings.
ChangeLog v2->v3:
- Put the example inside a dsi-controller so we have a complete
example that verifies to the DSI panel generic binding.
ChangeLog v1->v2:
- Suggest a stand-alone YAML bindings file for DSI panels in
a separate patch, and use that to reference the
boolean "enforce-video-mode" attribute for DSI panels
---
.../display/panel/sony,acx424akp.yaml | 49 +++
1 file changed, 49 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
diff --git
a/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
new file mode 100644
index ..a2f49b9a5958
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/sony,acx424akp.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/sony,acx424akp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony ACX424AKP 4" 480x864 AMOLED panel
+
+maintainers:
+ - Linus Walleij
+
+allOf:
+ - $ref: panel-common.yaml#
+ - $ref: ../dsi-controller.yaml#
+
+properties:
+ compatible:
+const: sony,acx424akp
+ reg: true
+ port: true
+ reset-gpios: true
+ vddi-supply:
+ description: regulator that supplies the vddi voltage
+ enforce-video-mode: true
+
+required:
+ - compatible
+ - reg
+ - port
+ - reset-gpios
+ - power-supply
+
+additionalProperties: false
+
+examples:
+ - |
+dsi-controller@0 {
+compatible = "foo";
+#address-cells = <1>;
+#size-cells = <0>;
+panel@0 {
+compatible = "sony,acx424akp";
+reg = <0>;
+vddi-supply = <&foo>;
+reset-gpios = <&foo_gpio 0 GPIO_ACTIVE_LOW>;
+};
+};
+
+...
\ No newline at end of file
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3 v4] drm/panel: Add driver for Sony ACX424AKP panel
The Sony ACX424AKP is a command/videomode DSI panel for
mobile devices. It is used on the ST-Ericsson HREF520
reference design. We support video mode by default, but
it is possible to switch the panel into command mode
by using the bool property "dsi-command-mode".
Signed-off-by: Linus Walleij
---
ChangeLog v3->v4:
- No changes just resending with the new binding updates.
ChangeLog v2->v3:
- No changes just resending with the new binding updates.
ChangeLog v1->v2:
- Fix up the ID read function to split into reading header,
version and ID, store the version in the struct.
- Get rid of a surplus semicolon found by the build robot
while rewriting the above code.
- Use unsigned int in probe() loop.
- Set vrefresh to 60Hz, as good as any, the measured vrefresh
in continous command mode is ~117 Hz.
- Use a different for() idiom while retrying to read ID
5 times.
- Drop the sync pulse setting, we are not using this, this
panel uses an event.
- Use the generic "enforce-video-mode" for video mode
enforcement.
---
drivers/gpu/drm/panel/Kconfig| 7 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-sony-acx424akp.c | 542 +++
include/video/mipi_display.h | 1 +
4 files changed, 551 insertions(+)
create mode 100644 drivers/gpu/drm/panel/panel-sony-acx424akp.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index f152bc4eeb53..959df5bea7d2 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -316,6 +316,13 @@ config DRM_PANEL_SITRONIX_ST7789V
Say Y here if you want to enable support for the Sitronix
ST7789V controller for 240x320 LCD panels
+config DRM_PANEL_SONY_ACX424AKP
+ tristate "Sony ACX424AKP DSI command mode panel"
+ depends on OF
+ depends on DRM_MIPI_DSI
+ depends on BACKLIGHT_CLASS_DEVICE
+ select VIDEOMODE_HELPERS
+
config DRM_PANEL_SONY_ACX565AKM
tristate "Sony ACX565AKM panel"
depends on GPIOLIB && OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index b6cd39fe0f20..0b51793e3b43 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_DRM_PANEL_SHARP_LS037V7DW01) +=
panel-sharp-ls037v7dw01.o
obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o
obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
+obj-$(CONFIG_DRM_PANEL_SONY_ACX424AKP) += panel-sony-acx424akp.o
obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
obj-$(CONFIG_DRM_PANEL_TPO_TD028TTEC1) += panel-tpo-td028ttec1.o
obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c
b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
new file mode 100644
index ..96a1450b699e
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
@@ -0,0 +1,542 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * MIPI-DSI Sony ACX424AKP panel driver. This is a 480x864
+ * AMOLED panel with a command-only DSI interface.
+ *
+ * Copyright (C) Linaro Ltd. 2019
+ * Author: Linus Walleij
+ * Based on code and know-how from Marcus Lorentzon
+ * Copyright (C) ST-Ericsson SA 2010
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define ACX424_DCS_READ_ID10xDA
+#define ACX424_DCS_READ_ID20xDB
+#define ACX424_DCS_READ_ID30xDC
+
+#define DISPLAY_SONY_ACX424AKP_ID1 0x811b
+#define DISPLAY_SONY_ACX424AKP_ID2 0x811a
+#define DISPLAY_SONY_ACX424AKP_ID3 0x8000
+
+struct acx424akp {
+ struct device *dev;
+ struct drm_panel panel;
+ struct backlight_device *bl;
+ struct regulator *supply;
+ struct gpio_desc *reset_gpio;
+ u8 version;
+ bool video_mode;
+};
+
+static const struct drm_display_mode sony_acx424akp_vid_mode = {
+ .clock = 33,
+ .hdisplay = 480,
+ .hsync_start = 480 + 15,
+ .hsync_end = 480 + 15 + 0,
+ .htotal = 480 + 15 + 0 + 15,
+ .vdisplay = 864,
+ .vsync_start = 864 + 14,
+ .vsync_end = 864 + 14 + 1,
+ .vtotal = 864 + 14 + 1 + 11,
+ .vrefresh = 60,
+ .width_mm = 48,
+ .height_mm = 84,
+ .flags = DRM_MODE_FLAG_PVSYNC,
+};
+
+/*
+ * The timings are not very helpful as the display is used in
+ * command mode using the maximum HS frequency.
+ */
+static const struct drm_display_mode sony_acx424akp_cmd_mode = {
+ .clock = 420160,
+ .hdisplay = 480,
+ .hsync_start = 480 + 154,
+ .hsync_end = 480 + 154 + 16,
+ .htotal = 480 + 154 + 16 + 32,
+ .vdisplay = 864,
+ .vsync_start = 864 + 1,
+ .vsync_end = 864 + 1 + 1,
+ .vtotal = 864 + 1 + 1 + 1,
+ /*
+* Some de
Re: [Intel-gfx] [PATCH 1/2] drm/property: Enforce more lifetime rules
On Thu, Oct 24, 2019 at 12:43 PM Thierry Reding
wrote:
>
> On Thu, Oct 24, 2019 at 12:40:55PM +0200, Thierry Reding wrote:
> > On Wed, Oct 23, 2019 at 04:49:52PM +0200, Daniel Vetter wrote:
> > > Properties can't be attached after registering, userspace would get
> > > confused (no one bothers to reprobe really).
> > >
> > > - Add kerneldoc
> > > - Enforce this with some checks. This needs a somewhat ugly check
> > > since connectors can be added later on, but we still need to attach
> > > all properties before they go public.
> > >
> > > Note that we already enforce that properties themselves are created
> > > before the entire device is registered.
> > >
> > > Cc: Jani Nikula
> > > Cc: Rajat Jain
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > drivers/gpu/drm/drm_mode_object.c | 14 ++
> > > 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_mode_object.c
> > > b/drivers/gpu/drm/drm_mode_object.c
> > > index 6a23e36ed4fe..35c2719407a8 100644
> > > --- a/drivers/gpu/drm/drm_mode_object.c
> > > +++ b/drivers/gpu/drm/drm_mode_object.c
> > > @@ -224,12 +224,26 @@ EXPORT_SYMBOL(drm_mode_object_get);
> > > * This attaches the given property to the modeset object with the given
> > > initial
> > > * value. Currently this function cannot fail since the properties are
> > > stored in
> > > * a statically sized array.
> > > + *
> > > + * Note that all properties must be attached before the object itself is
> > > + * registered and accessible from userspace.
> > > */
> > > void drm_object_attach_property(struct drm_mode_object *obj,
> > > struct drm_property *property,
> > > uint64_t init_val)
> > > {
> > > int count = obj->properties->count;
> > > + struct drm_device *dev = property->dev;
> > > +
> > > +
> > > + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
> > > + struct drm_connector *connector = obj_to_connector(obj);
> > > +
> > > + WARN_ON(!dev->driver->load &&
> > > + connector->registration_state ==
> > > DRM_CONNECTOR_REGISTERED);
> > > + } else {
> > > + WARN_ON(!dev->driver->load && dev->registered);
> > > + }
> >
> > I'm not sure I understand why dev->driver->load needs to be a special
> > case. Don't the same rules apply for those drivers as well?
>
> Nevermind, I just noticed that drm_dev_register() sets dev->registered
> to true before calling the driver's ->load() implementation, so makes
> sense:
We tried to be clever with this already before, see:
commit e0f32f78e51b9989ee89f608fd0dd10e9c230652 (tag:
drm-misc-next-fixes-2019-09-18)
Author: Daniel Vetter
Date: Tue Sep 17 14:09:35 2019 +0200
drm/kms: Duct-tape for mode object lifetime checks
commit 4f5368b5541a902f6596558b05f5c21a9770dd32
Author: Daniel Vetter
Date: Fri Jun 14 08:17:23 2019 +0200
drm/kms: Catch mode_object lifetime errors
uncovered a bit a mess in dp drivers. Most drivers (from a quick look,
all except i915) register all the dp stuff in their init code, which
is too early. With CONFIG_DRM_DP_AUX_CHARDEV this will blow up,
because drm_dp_aux_register tries to add a child to a device in sysfs
(the connector) which doesn't even exist yet.
No one seems to have cared thus far. But with the above change I also
moved the setting of dev->registered after the ->load callback, in an
attempt to keep old drivers from hitting any WARN_ON backtraces. But
that moved radeon.ko from the "working, by accident" to "now also
broken" category.
Since this is a huge mess I figured a revert would be simplest. But
this check has already caught issues in i915:
commit 1b9bd09630d4db4827cc04d358a41a16a6bc2cb0
Author: Ville Syrjälä
Date: Tue Aug 20 19:16:57 2019 +0300
drm/i915: Do not create a new max_bpc prop for MST connectors
Hence I'd like to retain it. Fix the radeon regression by moving the
setting of dev->registered back to were it was, and stop the
backtraces with an explicit check for dev->driver->load.
Everyone else will stay as broken with CONFIG_DRM_DP_AUX_CHARDEV. The
next patch will improve the kerneldoc and add a todo entry for this.
Fixes: 4f5368b5541a ("drm/kms: Catch mode_object lifetime errors")
Cc: Sean Paul
Cc: Maarten Lankhorst
Reported-by: Michel Dänzer
Reviewed-by: Michel Dänzer
Tested-by: Michel Dänzer
Cc: Michel Dänzer
Signed-off-by: Daniel Vetter
Link:
https://patchwork.freedesktop.org/patch/msgid/20190917120936.7501-1-daniel.vet...@ffwll.ch>
I think I'll add a reference to the above to the commit message when applying.
> Reviewed-by: Thierry Reding
Thanks for taking a look.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.o
Re: [PATCH] drm/syncobj: extend syncobj query ability v3
On Wed, Oct 23, 2019 at 10:22 PM Sean Paul wrote:
>
> On Tue, Jul 30, 2019 at 9:22 AM Chunming Zhou wrote:
> >
> >
> > 在 2019/7/30 21:17, Koenig, Christian 写道:
> > > Am 30.07.19 um 15:02 schrieb Chunming Zhou:
> > >> user space needs a flexiable query ability.
> > >> So that umd can get last signaled or submitted point.
> > >> v2:
> > >> add sanitizer checking.
> > >> v3:
> > >> rebase
> > >>
> > >> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> > >> Signed-off-by: Chunming Zhou
> > >> Cc: Lionel Landwerlin
> > >> Cc: Christian König
> > >> Reviewed-by: Lionel Landwerlin
> > > Reviewed-by: Christian König
> > >
> > > Lionel is the Intel code using this already public? Or David any chance
> > > that we can get a public amdvlk release using this?
> >
> > In latest public amdvlk, We should be able to see how timeline syncobj
> > is used in it.
> >
>
> I couldn't find this anywhere, could you please provide a link?
I thought there was also a radv implementation somewhere ... adding
some of the usual suspects for that.
-Daniel
>
> Sean
>
> >
> > -David
> >
> > >
> > > Christian.
> > >
> > >> ---
> > >>drivers/gpu/drm/drm_syncobj.c | 37 +--
> > >>include/uapi/drm/drm.h| 3 ++-
> > >>2 files changed, 24 insertions(+), 16 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_syncobj.c
> > >> b/drivers/gpu/drm/drm_syncobj.c
> > >> index cecff2e447b1..d4432f1513ac 100644
> > >> --- a/drivers/gpu/drm/drm_syncobj.c
> > >> +++ b/drivers/gpu/drm/drm_syncobj.c
> > >> @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct
> > >> drm_device *dev, void *data,
> > >> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> > >> return -EOPNOTSUPP;
> > >>
> > >> -if (args->pad != 0)
> > >> +if (args->flags != 0)
> > >> return -EINVAL;
> > >>
> > >> if (args->count_handles == 0)
> > >> @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device
> > >> *dev, void *data,
> > >> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> > >> return -EOPNOTSUPP;
> > >>
> > >> -if (args->pad != 0)
> > >> +if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
> > >> return -EINVAL;
> > >>
> > >> if (args->count_handles == 0)
> > >> @@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device
> > >> *dev, void *data,
> > >> fence = drm_syncobj_fence_get(syncobjs[i]);
> > >> chain = to_dma_fence_chain(fence);
> > >> if (chain) {
> > >> -struct dma_fence *iter, *last_signaled = NULL;
> > >> -
> > >> -dma_fence_chain_for_each(iter, fence) {
> > >> -if (iter->context != fence->context) {
> > >> -dma_fence_put(iter);
> > >> -/* It is most likely that timeline
> > >> has
> > >> - * unorder points. */
> > >> -break;
> > >> +struct dma_fence *iter, *last_signaled =
> > >> +dma_fence_get(fence);
> > >> +
> > >> +if (args->flags &
> > >> +DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> > >> +point = fence->seqno;
> > >> +} else {
> > >> +dma_fence_chain_for_each(iter, fence) {
> > >> +if (iter->context !=
> > >> fence->context) {
> > >> +dma_fence_put(iter);
> > >> +/* It is most likely that
> > >> timeline has
> > >> +* unorder points. */
> > >> +break;
> > >> +}
> > >> +dma_fence_put(last_signaled);
> > >> +last_signaled = dma_fence_get(iter);
> > >> }
> > >> -dma_fence_put(last_signaled);
> > >> -last_signaled = dma_fence_get(iter);
> > >> +point =
> > >> dma_fence_is_signaled(last_signaled) ?
> > >> +last_signaled->seqno :
> > >> +
> > >> to_dma_fence_chain(last_signaled)->prev_seqno;
> > >> }
> > >> -point = dma_fence_is_signaled(last_signaled) ?
> > >> -last_signaled->seqno :
> > >> -
> > >> to_dma_fence_chain(last_signaled)->prev_seqno;
> > >> dma_fence_put(last_signaled);
> > >> } else {
> > >> point = 0;
> > >> }
> > >> +dma_fence_put(fence);
> > >>
Re: [PATCH v1 1/3] gpu: host1x: Remove implicit IOMMU backing on client's registration
On Sun, Jun 23, 2019 at 08:37:41PM +0300, Dmitry Osipenko wrote:
> On ARM32 we don't want any of the clients device to be backed by the
> implicit domain, simply because we can't afford such a waste on older
> Tegra SoCs that have very few domains available in total. The recent IOMMU
> support addition for the Video Decoder hardware uncovered the problem
> that an unfortunate drivers probe order results in the DRM driver probe
> failure if CONFIG_ARM_DMA_USE_IOMMU=y due to a shortage of IOMMU domains
> caused by the implicit backing. The host1x_client_register() is a common
> function that is invoked by all of the relevant DRM drivers during theirs
> probe and hence it is convenient to remove the implicit backing there,
> resolving the problem.
>
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/gpu/host1x/bus.c | 19 +++
> 1 file changed, 19 insertions(+)
I don't really want to do this in a central place like this. If we
really do need this, why can't we do it in the individual drivers? Also,
we already call host1x_client_iommu_attach() from all the drivers and
that detaches from the IOMMU as well. So I'm not sure I understand why
this is needed.
Thierry
>
> diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> index 742aa9ff21b8..559df3974afb 100644
> --- a/drivers/gpu/host1x/bus.c
> +++ b/drivers/gpu/host1x/bus.c
> @@ -14,6 +14,10 @@
> #include "bus.h"
> #include "dev.h"
>
> +#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
> +#include
> +#endif
> +
> static DEFINE_MUTEX(clients_lock);
> static LIST_HEAD(clients);
>
> @@ -710,6 +714,21 @@ int host1x_client_register(struct host1x_client *client)
> struct host1x *host1x;
> int err;
>
> +#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
> + /*
> + * The client's driver could be backed by implicit IOMMU mapping
> + * and we don't want to have that because all of current Tegra
> + * drivers are managing IOMMU by themselves. This is a convenient
> + * place for unmapping of the implicit mapping because this function
> + * is called by all host1x drivers during theirs probe.
> + */
> + if (client->dev->archdata.mapping) {
> + struct dma_iommu_mapping *mapping =
> + to_dma_iommu_mapping(client->dev);
> + arm_iommu_detach_device(client->dev);
> + arm_iommu_release_mapping(mapping);
> + }
> +#endif
> mutex_lock(&devices_lock);
>
> list_for_each_entry(host1x, &devices, list) {
> --
> 2.22.0
>
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/simple-kms: Standardize arguments for callbacks
On Wed, Oct 23, 2019 at 05:40:32PM +0200, Linus Walleij wrote: > On Wed, Oct 23, 2019 at 12:13 PM Daniel Vetter wrote: > > > Passing the wrong type feels icky, everywhere else we use the pipe as > > the first parameter. Spotted while discussing patches with Thomas > > Zimmermann. > > > > v2: Make xen compile correctly > > > > Acked-By: Thomas Zimmermann (v1) > > Cc: Thomas Zimmermann > > Cc: Noralf Trønnes > > Cc: Gerd Hoffmann > > Cc: Eric Anholt > > Cc: Emil Velikov > > Cc: virtualizat...@lists.linux-foundation.org > > Cc: Linus Walleij > > Signed-off-by: Daniel Vetter > > Makes perfect sense. > Reviewed-by: Linus Walleij Thanks for taking a look, applied. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/3] drm/tegra: Fix 2d and 3d clients detaching from IOMMU domain
On Sun, Jun 23, 2019 at 08:37:42PM +0300, Dmitry Osipenko wrote:
> This should should fire up on the DRM's driver module re-loader because
> there won't be enough available domains on older Tegra SoCs.
>
> Cc: stable
> Fixes: 0c407de5ed1a ("drm/tegra: Refactor IOMMU attach/detach")
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/gpu/drm/tegra/dc.c | 4 ++--
> drivers/gpu/drm/tegra/drm.c | 9 ++---
> drivers/gpu/drm/tegra/drm.h | 3 ++-
> drivers/gpu/drm/tegra/gr2d.c | 4 ++--
> drivers/gpu/drm/tegra/gr3d.c | 4 ++--
> 5 files changed, 14 insertions(+), 10 deletions(-)
I think I understand what this is trying to do, but the commit message
does not help at all. So what's really going on here is that we need to
detach the device from the group regardless of whether we're sharing the
group or not, just like we attach groups to the shared domain whether
they share the same group or not.
But in that case, I wonder if it's even worth splitting groups the way
we are right now. Wouldn't it be better to just put all the devices into
the same group and be done with it?
The current code gives me headaches every time I read it, so if we can
just make it so that all the devices under the DRM device share the same
group, this would become a lot easier to deal with. I'm not really
convinced that it makes much sense to keep them on separate domains,
especially given the constraints on the number of domains available on
earlier Tegra devices.
Note that sharing a group will also make it much easier for these to use
the DMA API if it is backed by an IOMMU.
Let me see if I can throw something together to that effect.
Thierry
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index fa505bbc..c1b885444d90 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -2388,7 +2388,7 @@ static int tegra_dc_init(struct host1x_client *client)
> if (!IS_ERR(primary))
> drm_plane_cleanup(primary);
>
> - host1x_client_iommu_detach(client, dc->group);
> + host1x_client_iommu_detach(client, dc->group, true);
> host1x_syncpt_free(dc->syncpt);
>
> return err;
> @@ -2412,7 +2412,7 @@ static int tegra_dc_exit(struct host1x_client *client)
> return err;
> }
>
> - host1x_client_iommu_detach(client, dc->group);
> + host1x_client_iommu_detach(client, dc->group, true);
> host1x_syncpt_free(dc->syncpt);
>
> return 0;
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index d2080bd7d392..f94441457c64 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -1120,15 +1120,18 @@ struct iommu_group *host1x_client_iommu_attach(struct
> host1x_client *client,
> }
>
> void host1x_client_iommu_detach(struct host1x_client *client,
> - struct iommu_group *group)
> + struct iommu_group *group,
> + bool shared)
> {
> struct drm_device *drm = dev_get_drvdata(client->parent);
> struct tegra_drm *tegra = drm->dev_private;
>
> if (group) {
> - if (group == tegra->group) {
> + if (!shared || group == tegra->group) {
> iommu_detach_group(tegra->domain, group);
> - tegra->group = NULL;
> +
> + if (group == tegra->group)
> + tegra->group = NULL;
> }
>
> iommu_group_put(group);
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 488f36f00bd8..9f1a3d6f3406 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -107,7 +107,8 @@ int tegra_drm_unregister_client(struct tegra_drm *tegra,
> struct iommu_group *host1x_client_iommu_attach(struct host1x_client *client,
> bool shared);
> void host1x_client_iommu_detach(struct host1x_client *client,
> - struct iommu_group *group);
> + struct iommu_group *group,
> + bool shared);
>
> int tegra_drm_init(struct tegra_drm *tegra, struct drm_device *drm);
> int tegra_drm_exit(struct tegra_drm *tegra);
> diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
> index 673059fd2fcb..c486e0a05c9d 100644
> --- a/drivers/gpu/drm/tegra/gr2d.c
> +++ b/drivers/gpu/drm/tegra/gr2d.c
> @@ -69,7 +69,7 @@ static int gr2d_init(struct host1x_client *client)
> return 0;
>
> detach:
> - host1x_client_iommu_detach(client, gr2d->group);
> + host1x_client_iommu_detach(client, gr2d->group, false);
> free:
> host1x_syncpt_free(client->syncpts[0]);
> put:
> @@ -89,7 +89,7 @@ static int gr2d_exit(struct host1x_client *client)
> if (err < 0)
> return err;
>
> - host1x_client_iommu_detach(client, gr2d->group);
> + host1x_client_iommu_detach(client, gr
Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property
On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
> On Tue, 22 Oct 2019, Daniel Vetter wrote:
> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> This patch adds a scaling filter mode porperty
> >> to allow:
> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> - A userspace to pick a desired effect while scaling.
> >>
> >> This option will be particularly useful in the scenarios where
> >> Integer mode scaling is possible, and a UI client wants to pick
> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >>
> >> There was a RFC patch series published, to discus the request to enable
> >> Integer mode scaling by some of the gaming communities, which can be
> >> found here:
> >> https://patchwork.freedesktop.org/series/66175/
> >>
> >> Signed-off-by: Shashank Sharma
> >
> > Two things:
> >
> > - needs real property docs for this in the kms property chapter
> > - would be good if we could somehow subsume the panel fitter prop into
> > this? Or at least document possible interactions with that.
> >
> > Plus userspace ofc, recommended best practices is to add a Link: tag
> > pointing at the userspace patch series/merge request directly to the patch
> > that adds the new uapi.
>
> I think Link: should only be used for referencing the patch that became
> the commit?
Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
herd here. But yeah maybe we want something else.
>
> References: perhaps?
Or Userspace: to make it real obvious?
-Daniel
>
> >
> > Cheers, Daniel
> >> ---
> >> drivers/gpu/drm/drm_atomic_uapi.c | 4
> >> include/drm/drm_crtc.h| 26 ++
> >> include/drm/drm_mode_config.h | 6 ++
> >> 3 files changed, 36 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> >> b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 0d466d3b0809..883329453a86 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct
> >> drm_crtc *crtc,
> >>return ret;
> >>} else if (property == config->prop_vrr_enabled) {
> >>state->vrr_enabled = val;
> >> + } else if (property == config->scaling_filter_property) {
> >> + state->scaling_filter = val;
> >>} else if (property == config->degamma_lut_property) {
> >>ret = drm_atomic_replace_property_blob_from_id(dev,
> >>&state->degamma_lut,
> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >>*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >>else if (property == config->prop_out_fence_ptr)
> >>*val = 0;
> >> + else if (property == config->scaling_filter_property)
> >> + *val = state->scaling_filter;
> >>else if (crtc->funcs->atomic_get_property)
> >>return crtc->funcs->atomic_get_property(crtc, state, property,
> >> val);
> >>else
> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> --- a/include/drm/drm_crtc.h
> >> +++ b/include/drm/drm_crtc.h
> >> @@ -58,6 +58,25 @@ struct device_node;
> >> struct dma_fence;
> >> struct edid;
> >>
> >> +enum drm_scaling_filters {
> >> + DRM_SCALING_FILTER_DEFAULT,
> >> + DRM_SCALING_FILTER_MEDIUM,
> >> + DRM_SCALING_FILTER_BILINEAR,
> >> + DRM_SCALING_FILTER_NN,
> >> + DRM_SCALING_FILTER_NN_IS_ONLY,
> >> + DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> + DRM_SCALING_FILTER_INVALID,
> >> +};
> >> +
> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> >> + { DRM_SCALING_FILTER_DEFAULT, "Default" },
> >> + { DRM_SCALING_FILTER_MEDIUM, "Medium" },
> >> + { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> >> + { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> >> + { DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> >> + { DRM_SCALING_FILTER_INVALID, "Invalid" },
> >> +};
> >> +
> >> static inline int64_t U642I64(uint64_t val)
> >> {
> >>return (int64_t)*((int64_t *)&val);
> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >> */
> >>u32 target_vblank;
> >>
> >> + /**
> >> + * @scaling_filter:
> >> + *
> >> + * Scaling filter mode to be applied
> >> + */
> >> + u32 scaling_filter;
> >> +
> >>/**
> >> * @async_flip:
> >> *
> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> >> index 3bcbe30339f0..efd6fd55770f 100644
> >> --- a/include/drm/drm_mode_config.h
> >> +++ b/include/drm/drm_mode_config.h
> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
> >> */
> >>struct drm_property *modifiers_property;
> >>
> >> + /**
> >> + * @scaling_filter_property: CRTC property to apply a particular filter
> >> + * while scaling in panel fitter mode.
> >> + */
> >> + struct drm_property *scaling_filter_property;
> >> +
> >>/*
Re: [PATCH 1/2] drm/property: Enforce more lifetime rules
On Wed, 23 Oct 2019, Daniel Vetter wrote:
> Properties can't be attached after registering, userspace would get
> confused (no one bothers to reprobe really).
>
> - Add kerneldoc
> - Enforce this with some checks. This needs a somewhat ugly check
> since connectors can be added later on, but we still need to attach
> all properties before they go public.
>
> Note that we already enforce that properties themselves are created
> before the entire device is registered.
>
> Cc: Jani Nikula
> Cc: Rajat Jain
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_mode_object.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mode_object.c
> b/drivers/gpu/drm/drm_mode_object.c
> index 6a23e36ed4fe..35c2719407a8 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -224,12 +224,26 @@ EXPORT_SYMBOL(drm_mode_object_get);
> * This attaches the given property to the modeset object with the given
> initial
> * value. Currently this function cannot fail since the properties are
> stored in
> * a statically sized array.
> + *
> + * Note that all properties must be attached before the object itself is
> + * registered and accessible from userspace.
> */
> void drm_object_attach_property(struct drm_mode_object *obj,
> struct drm_property *property,
> uint64_t init_val)
> {
> int count = obj->properties->count;
> + struct drm_device *dev = property->dev;
> +
> +
Superfluous newline, with that fixed,
Reviewed-by: Jani Nikula
> + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
> + struct drm_connector *connector = obj_to_connector(obj);
> +
> + WARN_ON(!dev->driver->load &&
> + connector->registration_state ==
> DRM_CONNECTOR_REGISTERED);
> + } else {
> + WARN_ON(!dev->driver->load && dev->registered);
> + }
>
> if (count == DRM_OBJECT_MAX_PROPERTY) {
> WARN(1, "Failed to attach object property (type: 0x%x). Please "
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 0/9] backlight: gpio: simplify the driver
On Thu, 24 Oct 2019, Jacopo Mondi wrote: > On Thu, Oct 24, 2019 at 07:47:26AM +0100, Lee Jones wrote: > > On Wed, 23 Oct 2019, Daniel Thompson wrote: [...] > > > > Jacopo is travelling until November 1st and won't be able to test this > > > > again before this date. Do you think you can pick it up and in case > > > > anything's broken on SH, we can fix it after v5.5-rc1, so that it > > > > doesn't miss another merge window? > > > > November 1st (-rc6) will be fine. > > > > I'd rather apply it late-tested than early-non-tested. > > > > Hopefully Jacopo can prioritise testing this on Thursday or Friday, > > since Monday will be -rc7 which is really cutting it fine. > > I'll do my best, I'll get home Friday late afternoon :) Thanks. We'd all really appreciate it. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property
On Thu, 24 Oct 2019, Daniel Vetter wrote:
> On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
>> On Tue, 22 Oct 2019, Daniel Vetter wrote:
>> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> >> This patch adds a scaling filter mode porperty
>> >> to allow:
>> >> - A driver/HW to showcase it's scaling filter capabilities.
>> >> - A userspace to pick a desired effect while scaling.
>> >>
>> >> This option will be particularly useful in the scenarios where
>> >> Integer mode scaling is possible, and a UI client wants to pick
>> >> filters like Nearest-neighbor applied for non-blurry outputs.
>> >>
>> >> There was a RFC patch series published, to discus the request to enable
>> >> Integer mode scaling by some of the gaming communities, which can be
>> >> found here:
>> >> https://patchwork.freedesktop.org/series/66175/
>> >>
>> >> Signed-off-by: Shashank Sharma
>> >
>> > Two things:
>> >
>> > - needs real property docs for this in the kms property chapter
>> > - would be good if we could somehow subsume the panel fitter prop into
>> > this? Or at least document possible interactions with that.
>> >
>> > Plus userspace ofc, recommended best practices is to add a Link: tag
>> > pointing at the userspace patch series/merge request directly to the patch
>> > that adds the new uapi.
>>
>> I think Link: should only be used for referencing the patch that became
>> the commit?
>
> Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
> herd here. But yeah maybe we want something else.
>>
>> References: perhaps?
>
> Or Userspace: to make it real obvious?
Works for me, as long as we don't conflate Link. (Maybe Link: should've
been Patch: or Submission: or whatever.)
BR,
Jani.
> -Daniel
>
>>
>> >
>> > Cheers, Daniel
>> >> ---
>> >> drivers/gpu/drm/drm_atomic_uapi.c | 4
>> >> include/drm/drm_crtc.h| 26 ++
>> >> include/drm/drm_mode_config.h | 6 ++
>> >> 3 files changed, 36 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
>> >> b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> index 0d466d3b0809..883329453a86 100644
>> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct
>> >> drm_crtc *crtc,
>> >> return ret;
>> >> } else if (property == config->prop_vrr_enabled) {
>> >> state->vrr_enabled = val;
>> >> + } else if (property == config->scaling_filter_property) {
>> >> + state->scaling_filter = val;
>> >> } else if (property == config->degamma_lut_property) {
>> >> ret = drm_atomic_replace_property_blob_from_id(dev,
>> >> &state->degamma_lut,
>> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>> >> *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>> >> else if (property == config->prop_out_fence_ptr)
>> >> *val = 0;
>> >> + else if (property == config->scaling_filter_property)
>> >> + *val = state->scaling_filter;
>> >> else if (crtc->funcs->atomic_get_property)
>> >> return crtc->funcs->atomic_get_property(crtc, state, property,
>> >> val);
>> >> else
>> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> >> index 5e9b15a0e8c5..94c5509474a8 100644
>> >> --- a/include/drm/drm_crtc.h
>> >> +++ b/include/drm/drm_crtc.h
>> >> @@ -58,6 +58,25 @@ struct device_node;
>> >> struct dma_fence;
>> >> struct edid;
>> >>
>> >> +enum drm_scaling_filters {
>> >> + DRM_SCALING_FILTER_DEFAULT,
>> >> + DRM_SCALING_FILTER_MEDIUM,
>> >> + DRM_SCALING_FILTER_BILINEAR,
>> >> + DRM_SCALING_FILTER_NN,
>> >> + DRM_SCALING_FILTER_NN_IS_ONLY,
>> >> + DRM_SCALING_FILTER_EDGE_ENHANCE,
>> >> + DRM_SCALING_FILTER_INVALID,
>> >> +};
>> >> +
>> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> >> + { DRM_SCALING_FILTER_DEFAULT, "Default" },
>> >> + { DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> >> + { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> >> + { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> >> + { DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> >> + { DRM_SCALING_FILTER_INVALID, "Invalid" },
>> >> +};
>> >> +
>> >> static inline int64_t U642I64(uint64_t val)
>> >> {
>> >> return (int64_t)*((int64_t *)&val);
>> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>> >>*/
>> >> u32 target_vblank;
>> >>
>> >> + /**
>> >> + * @scaling_filter:
>> >> + *
>> >> + * Scaling filter mode to be applied
>> >> + */
>> >> + u32 scaling_filter;
>> >> +
>> >> /**
>> >>* @async_flip:
>> >>*
>> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> >> index 3bcbe30339f0..efd6fd55770f 100644
>> >> --- a/include/drm/drm_mode_config.h
>> >> +++ b/include/drm/drm_mode_config.h
>> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
>> >>*/
>> >> struct drm_pro
Re: [Outreachy][PATCH] drm/mediatek: remove cast to pointers passed to kfree
On Wed, Oct 23, 2019 at 02:11:07PM +0300, Wambui Karuga wrote: > Remove unnecessary casts to pointer types passed to kfree. > Issue detected by coccinelle: > @@ > type t1; > expression *e; > @@ > > -kfree((t1 *)e); > +kfree(e); > > Signed-off-by: Wambui Karuga Applied to drm-misc-next, thanks for your patch. -Daniel > --- > drivers/gpu/drm/mediatek/mtk_drm_gem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > index ca672f1d140d..b04a3c2b111e 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > @@ -271,7 +271,7 @@ void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj) > pgprot_writecombine(PAGE_KERNEL)); > > out: > - kfree((void *)sgt); > + kfree(sgt); > > return mtk_gem->kvaddr; > } > @@ -285,5 +285,5 @@ void mtk_drm_gem_prime_vunmap(struct drm_gem_object *obj, > void *vaddr) > > vunmap(vaddr); > mtk_gem->kvaddr = 0; > - kfree((void *)mtk_gem->pages); > + kfree(mtk_gem->pages); > } > -- > 2.23.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property
On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
> On Thu, 24 Oct 2019, Daniel Vetter wrote:
> > On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
> >> On Tue, 22 Oct 2019, Daniel Vetter wrote:
> >> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> >> This patch adds a scaling filter mode porperty
> >> >> to allow:
> >> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> >> - A userspace to pick a desired effect while scaling.
> >> >>
> >> >> This option will be particularly useful in the scenarios where
> >> >> Integer mode scaling is possible, and a UI client wants to pick
> >> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >> >>
> >> >> There was a RFC patch series published, to discus the request to enable
> >> >> Integer mode scaling by some of the gaming communities, which can be
> >> >> found here:
> >> >> https://patchwork.freedesktop.org/series/66175/
> >> >>
> >> >> Signed-off-by: Shashank Sharma
> >> >
> >> > Two things:
> >> >
> >> > - needs real property docs for this in the kms property chapter
> >> > - would be good if we could somehow subsume the panel fitter prop into
> >> > this? Or at least document possible interactions with that.
> >> >
> >> > Plus userspace ofc, recommended best practices is to add a Link: tag
> >> > pointing at the userspace patch series/merge request directly to the
> >> > patch
> >> > that adds the new uapi.
> >>
> >> I think Link: should only be used for referencing the patch that became
> >> the commit?
> >
> > Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
> > herd here. But yeah maybe we want something else.
> >>
> >> References: perhaps?
> >
> > Or Userspace: to make it real obvious?
>
> Works for me, as long as we don't conflate Link. (Maybe Link: should've
> been Patch: or Submission: or whatever.)
The Powers That Be (kernel summit) decided that it's Link for reference to
a http link of an archive of some sort that contains the msg-id.
Aside: I asked k.o admins to add dri-devel/amdgpu-dev/intel-gfx to
lore.k.o. They'll backfill the entire archive too. Imo still better we
point at patchwork, since that's where hopefully the CI result hangs out.
-Daniel
>
> BR,
> Jani.
>
>
> > -Daniel
> >
> >>
> >> >
> >> > Cheers, Daniel
> >> >> ---
> >> >> drivers/gpu/drm/drm_atomic_uapi.c | 4
> >> >> include/drm/drm_crtc.h| 26 ++
> >> >> include/drm/drm_mode_config.h | 6 ++
> >> >> 3 files changed, 36 insertions(+)
> >> >>
> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> b/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> index 0d466d3b0809..883329453a86 100644
> >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct
> >> >> drm_crtc *crtc,
> >> >> return ret;
> >> >> } else if (property == config->prop_vrr_enabled) {
> >> >> state->vrr_enabled = val;
> >> >> + } else if (property == config->scaling_filter_property) {
> >> >> + state->scaling_filter = val;
> >> >> } else if (property == config->degamma_lut_property) {
> >> >> ret = drm_atomic_replace_property_blob_from_id(dev,
> >> >> &state->degamma_lut,
> >> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >> >> *val = (state->gamma_lut) ? state->gamma_lut->base.id :
> >> >> 0;
> >> >> else if (property == config->prop_out_fence_ptr)
> >> >> *val = 0;
> >> >> + else if (property == config->scaling_filter_property)
> >> >> + *val = state->scaling_filter;
> >> >> else if (crtc->funcs->atomic_get_property)
> >> >> return crtc->funcs->atomic_get_property(crtc, state,
> >> >> property, val);
> >> >> else
> >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> >> --- a/include/drm/drm_crtc.h
> >> >> +++ b/include/drm/drm_crtc.h
> >> >> @@ -58,6 +58,25 @@ struct device_node;
> >> >> struct dma_fence;
> >> >> struct edid;
> >> >>
> >> >> +enum drm_scaling_filters {
> >> >> + DRM_SCALING_FILTER_DEFAULT,
> >> >> + DRM_SCALING_FILTER_MEDIUM,
> >> >> + DRM_SCALING_FILTER_BILINEAR,
> >> >> + DRM_SCALING_FILTER_NN,
> >> >> + DRM_SCALING_FILTER_NN_IS_ONLY,
> >> >> + DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> >> + DRM_SCALING_FILTER_INVALID,
> >> >> +};
> >> >> +
> >> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[]
> >> >> = {
> >> >> + { DRM_SCALING_FILTER_DEFAULT, "Default" },
> >> >> + { DRM_SCALING_FILTER_MEDIUM, "Medium" },
> >> >> + { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> >> >> + { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>
Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property
On Thu, 24 Oct 2019, Daniel Vetter wrote:
> On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
>> On Thu, 24 Oct 2019, Daniel Vetter wrote:
>> > On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
>> >> On Tue, 22 Oct 2019, Daniel Vetter wrote:
>> >> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> >> >> This patch adds a scaling filter mode porperty
>> >> >> to allow:
>> >> >> - A driver/HW to showcase it's scaling filter capabilities.
>> >> >> - A userspace to pick a desired effect while scaling.
>> >> >>
>> >> >> This option will be particularly useful in the scenarios where
>> >> >> Integer mode scaling is possible, and a UI client wants to pick
>> >> >> filters like Nearest-neighbor applied for non-blurry outputs.
>> >> >>
>> >> >> There was a RFC patch series published, to discus the request to enable
>> >> >> Integer mode scaling by some of the gaming communities, which can be
>> >> >> found here:
>> >> >> https://patchwork.freedesktop.org/series/66175/
>> >> >>
>> >> >> Signed-off-by: Shashank Sharma
>> >> >
>> >> > Two things:
>> >> >
>> >> > - needs real property docs for this in the kms property chapter
>> >> > - would be good if we could somehow subsume the panel fitter prop into
>> >> > this? Or at least document possible interactions with that.
>> >> >
>> >> > Plus userspace ofc, recommended best practices is to add a Link: tag
>> >> > pointing at the userspace patch series/merge request directly to the
>> >> > patch
>> >> > that adds the new uapi.
>> >>
>> >> I think Link: should only be used for referencing the patch that became
>> >> the commit?
>> >
>> > Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
>> > herd here. But yeah maybe we want something else.
>> >>
>> >> References: perhaps?
>> >
>> > Or Userspace: to make it real obvious?
>>
>> Works for me, as long as we don't conflate Link. (Maybe Link: should've
>> been Patch: or Submission: or whatever.)
>
> The Powers That Be (kernel summit) decided that it's Link for reference to
> a http link of an archive of some sort that contains the msg-id.
Oh totally aware of that. But hopefully they also assert that Link is
not also for something else.
> Aside: I asked k.o admins to add dri-devel/amdgpu-dev/intel-gfx to
> lore.k.o. They'll backfill the entire archive too. Imo still better we
> point at patchwork, since that's where hopefully the CI result hangs out.
Thanks, appreciated. Agreed on patchwork.
BR,
Jani.
> -Daniel
>
>>
>> BR,
>> Jani.
>>
>>
>> > -Daniel
>> >
>> >>
>> >> >
>> >> > Cheers, Daniel
>> >> >> ---
>> >> >> drivers/gpu/drm/drm_atomic_uapi.c | 4
>> >> >> include/drm/drm_crtc.h| 26 ++
>> >> >> include/drm/drm_mode_config.h | 6 ++
>> >> >> 3 files changed, 36 insertions(+)
>> >> >>
>> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> index 0d466d3b0809..883329453a86 100644
>> >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct
>> >> >> drm_crtc *crtc,
>> >> >>return ret;
>> >> >>} else if (property == config->prop_vrr_enabled) {
>> >> >>state->vrr_enabled = val;
>> >> >> + } else if (property == config->scaling_filter_property) {
>> >> >> + state->scaling_filter = val;
>> >> >>} else if (property == config->degamma_lut_property) {
>> >> >>ret = drm_atomic_replace_property_blob_from_id(dev,
>> >> >>&state->degamma_lut,
>> >> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>> >> >>*val = (state->gamma_lut) ? state->gamma_lut->base.id :
>> >> >> 0;
>> >> >>else if (property == config->prop_out_fence_ptr)
>> >> >>*val = 0;
>> >> >> + else if (property == config->scaling_filter_property)
>> >> >> + *val = state->scaling_filter;
>> >> >>else if (crtc->funcs->atomic_get_property)
>> >> >>return crtc->funcs->atomic_get_property(crtc, state,
>> >> >> property, val);
>> >> >>else
>> >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> >> >> index 5e9b15a0e8c5..94c5509474a8 100644
>> >> >> --- a/include/drm/drm_crtc.h
>> >> >> +++ b/include/drm/drm_crtc.h
>> >> >> @@ -58,6 +58,25 @@ struct device_node;
>> >> >> struct dma_fence;
>> >> >> struct edid;
>> >> >>
>> >> >> +enum drm_scaling_filters {
>> >> >> + DRM_SCALING_FILTER_DEFAULT,
>> >> >> + DRM_SCALING_FILTER_MEDIUM,
>> >> >> + DRM_SCALING_FILTER_BILINEAR,
>> >> >> + DRM_SCALING_FILTER_NN,
>> >> >> + DRM_SCALING_FILTER_NN_IS_ONLY,
>> >> >> + DRM_SCALING_FILTER_EDGE_ENHANCE,
>> >> >> + DRM_SCALING_FILTER_INVALID,
>> >> >> +};
>> >> >> +
>> >> >> +static const struct drm_prop_
Re: [PATCH] Cleanup: replace prefered with preferred
On Wed, 23 Oct 2019, Mark Salyzyn wrote: > I will split this between pure and inert documentation/comments for now, > with a followup later for the code portion which understandably is more > controversial. Please split by driver/subsystem too, and it'll be all around much easier for everyone. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205277] [amd powerplay] vega10: soc voltage for power state 7 is not changed by overdrive.
https://bugzilla.kernel.org/show_bug.cgi?id=205277 stefansp...@gmail.com changed: What|Removed |Added CC||stefansp...@gmail.com --- Comment #4 from stefansp...@gmail.com --- (In reply to haro41 from comment #3) > Did you debug this issue? I think the problem could be outside this code. > > I would outcomment the if-statement following for-loop in your proposed > patch, because otherwise 'i' points outside the array boundarys here. I think the if statement is fine as both od_vddc_lookup_table->entries[] and podn_vdd_dep->entries[] both hold MAX_REGULAR_DPM_NUMBER members, which is 8, so accessing entries[7] is not out of bounds. Btw, the patch works for me aswell. Card behaves as it should after loading my pp_table, which was not the case before. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/prime: Fix mmap fake offset for drm_gem_object_funcs.mmap
On Thu, Oct 24, 2019 at 11:02:40AM +0200, Gerd Hoffmann wrote:
> On Wed, Oct 23, 2019 at 05:22:26PM -0500, Rob Herring wrote:
> > Commit c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs")
> > introduced a GEM object mmap() hook which is expected to subtract the
> > fake offset from vm_pgoff.
>
> Long-term it is probably a good idea to just remove the fake offset
> handling from drivers. But that'll only work once all drivers switched
> away from custom fops->mmap handlers so we can handle the offset -> obj
> lookup in the drm core for everybody.
>
> So let's go this way for now.
>
> Acked-by: Gerd Hoffmann
Uh this sounds like doubling down on rather horrible semantics. Can we at
least stop the mess instead of baking it in for real? The hook is very
very new after all. I.e.
- Document that obj->funcs->mmap will have 0 offset in the kerneldoc.
- Remove the subtracting from the shmem helper
- In ttm_bo_mmap_obj re-add the offset with a huge FIXME comment.
- Adjust drm_gem_mmap_obj to do that same for obj->funcs->mmap and also
document the expectation there too.
This feels like very much going the wrong direction ...
Also I guess Gerd didn't really test this prime mmap support?
Thanks, Daniel
>
> > However, for mmap() on dmabufs, there is not
> > a fake offset. To fix this, we need to add the fake offset just like the
> > driver->fops->mmap() code path.
> >
> > Fixes: c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs")
> > Cc: Gerd Hoffmann
> > Cc: Daniel Vetter
> > Signed-off-by: Rob Herring
> > ---
> > I ran into this while working on converting vgem to shmem helpers and
> > the IGT vgem_basic dmabuf-mmap test failed. This fixes shmem, but I
> > have checked any other users of the new mmap hook.
> > Rob
> >
> > drivers/gpu/drm/drm_prime.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > index 0814211b0f3f..5d06690a2e9d 100644
> > --- a/drivers/gpu/drm/drm_prime.c
> > +++ b/drivers/gpu/drm/drm_prime.c
> > @@ -713,6 +713,8 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj,
> > struct vm_area_struct *vma)
> > struct file *fil;
> > int ret;
> >
> > + vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> > +
> > if (obj->funcs && obj->funcs->mmap) {
> > ret = obj->funcs->mmap(obj, vma);
> > if (ret)
> > @@ -737,8 +739,6 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj,
> > struct vm_area_struct *vma)
> > if (ret)
> > goto out;
> >
> > - vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);
> > -
> > ret = obj->dev->driver->fops->mmap(fil, vma);
> >
> > drm_vma_node_revoke(&obj->vma_node, priv);
> > --
> > 2.20.1
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gpu: Add comment for memory barrier
On Thu, Oct 24, 2019 at 09:58:33AM +0530, Bhanusree wrote:
> -Add comment for memory barrier
> -Issue found using checkpatch.pl
>
> Signed-off-by: Bhanusree
> ---
>
> v2:modified memory barrier comments appropriately
Applied, thanks.
-Daniel
>
> drivers/gpu/drm/drm_cache.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
> index 3bd76e9..e574261 100644
> --- a/drivers/gpu/drm/drm_cache.c
> +++ b/drivers/gpu/drm/drm_cache.c
> @@ -62,10 +62,10 @@ static void drm_cache_flush_clflush(struct page *pages[],
> {
> unsigned long i;
>
> - mb();
> + mb(); /*Full memory barrier used before so that CLFLUSH is ordered*/
> for (i = 0; i < num_pages; i++)
> drm_clflush_page(*pages++);
> - mb();
> + mb(); /*Also used after CLFLUSH so that all cache is flushed*/
> }
> #endif
>
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/4] drm/vram: Provide helpers for prepare_fb() and cleanup_fb()
On Thu, Oct 24, 2019 at 10:14:00AM +0200, Thomas Zimmermann wrote: > The implementation of the plane's call-back functions prepare_fb() and > cleanup_fb() for GEM VRAM helpers are sharable among drivers. > > Patch #3 also fixes two bugs that have been present in hibmc since it was > first added. The primary plane's atomic_update() is not responsible for > pinning BOs. And it never unpinned unused BOs. VRAM is being exausted > over time. > > The new helpers have been tested to work. > > v2: > * provide helpers for struct drm_simple_display_pipe_funcs, and... > * ...use them in bochs Oh I thought we agreed on changing the simple_pipe type for prepare/cleanup_fb ... But this works too ofc. On the series: Reviewed-by: Daniel Vetter > > Thomas Zimmermann (4): > drm/vram-helpers: Add helpers for prepare_fb() and cleanup_fb() > drm/bochs: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers > drm/hisilicon/hibmc: Use GEM VRAM's prepare_fb() and cleanup_fb() > helpers > drm/vboxvideo: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers > > drivers/gpu/drm/bochs/bochs_kms.c | 26 +--- > drivers/gpu/drm/drm_gem_vram_helper.c | 126 ++ > .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 14 +- > drivers/gpu/drm/vboxvideo/vbox_mode.c | 61 + > include/drm/drm_gem_vram_helper.h | 25 > 5 files changed, 161 insertions(+), 91 deletions(-) > > -- > 2.23.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/v3d: fix double free of bin
On Thu, Oct 24, 2019 at 11:48:01AM +0100, Colin King wrote:
> From: Colin Ian King
>
> Two different fixes have addressed the same memory leak of bin and
> this now causes a double free of bin. While the individual memory
> leak fixes are fine, both fixes together are problematic.
>
> Addresses-Coverity: ("Double free")
> Fixes: 29cd13cfd762 ("drm/v3d: Fix memory leak in v3d_submit_cl_ioctl")
> Fixes: 0d352a3a8a1f (" rm/v3d: don't leak bin job if v3d_job_init fails.")
> Signed-off-by: Colin Ian King
That sounds like wrong merge resolution somewhere, and we don't have those
patches merged together in any final tree yet anywhere. What's this based
on?
-Daniel
> ---
> drivers/gpu/drm/v3d/v3d_gem.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
> index 549dde83408b..37515e47b47e 100644
> --- a/drivers/gpu/drm/v3d/v3d_gem.c
> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> @@ -568,7 +568,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
> ret = v3d_job_init(v3d, file_priv, &bin->base,
> v3d_job_free, args->in_sync_bcl);
> if (ret) {
> - kfree(bin);
> v3d_job_put(&render->base);
> kfree(bin);
> return ret;
> --
> 2.20.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: use the parent resv for ghost objects v3
On Thu, Oct 24, 2019 at 01:16:32PM +0200, Christian König wrote:
> This way the TTM is destroyed with the correct dma_resv object
> locked and we can even pipeline imported BO evictions.
>
> v2: Limit this to only cases when the parent object uses a separate
> reservation object as well. This fixes another OOM problem.
> v3: fix init and try_lock on the wrong object
Uh this makes a lot more sense with that fixed :-)
Now even feeling bold enough for a
Reviewed-by: Daniel Vetter
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/ttm/ttm_bo_util.c | 20 +++-
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 73a1b0186029..f7b57ca1a95b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -516,9 +516,11 @@ static int ttm_buffer_object_transfer(struct
> ttm_buffer_object *bo,
> kref_init(&fbo->base.kref);
> fbo->base.destroy = &ttm_transfered_destroy;
> fbo->base.acc_size = 0;
> - fbo->base.base.resv = &fbo->base.base._resv;
> - dma_resv_init(fbo->base.base.resv);
> - ret = dma_resv_trylock(fbo->base.base.resv);
> + if (bo->base.resv == &bo->base._resv)
> + fbo->base.base.resv = &fbo->base.base._resv;
> +
> + dma_resv_init(&fbo->base.base._resv);
> + ret = dma_resv_trylock(&fbo->base.base._resv);
> WARN_ON(!ret);
>
> *new_obj = &fbo->base;
> @@ -715,7 +717,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object
> *bo,
> if (ret)
> return ret;
>
> - dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
> + dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
>
> /**
>* If we're not moving to fixed memory, the TTM object
> @@ -728,7 +730,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object
> *bo,
> else
> bo->ttm = NULL;
>
> - ttm_bo_unreserve(ghost_obj);
> + dma_resv_unlock(&ghost_obj->base._resv);
> ttm_bo_put(ghost_obj);
> }
>
> @@ -771,7 +773,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
> if (ret)
> return ret;
>
> - dma_resv_add_excl_fence(ghost_obj->base.resv, fence);
> + dma_resv_add_excl_fence(&ghost_obj->base._resv, fence);
>
> /**
>* If we're not moving to fixed memory, the TTM object
> @@ -784,7 +786,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
> else
> bo->ttm = NULL;
>
> - ttm_bo_unreserve(ghost_obj);
> + dma_resv_unlock(&ghost_obj->base._resv);
> ttm_bo_put(ghost_obj);
>
> } else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
> @@ -840,7 +842,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
> if (ret)
> return ret;
>
> - ret = dma_resv_copy_fences(ghost->base.resv, bo->base.resv);
> + ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv);
> /* Last resort, wait for the BO to be idle when we are OOM */
> if (ret)
> ttm_bo_wait(bo, false, false);
> @@ -849,7 +851,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
> bo->mem.mem_type = TTM_PL_SYSTEM;
> bo->ttm = NULL;
>
> - ttm_bo_unreserve(ghost);
> + dma_resv_unlock(&ghost->base._resv);
> ttm_bo_put(ghost);
>
> return 0;
> --
> 2.17.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/v3d: fix double free of bin
On 24/10/2019 13:38, Daniel Vetter wrote:
> On Thu, Oct 24, 2019 at 11:48:01AM +0100, Colin King wrote:
>> From: Colin Ian King
>>
>> Two different fixes have addressed the same memory leak of bin and
>> this now causes a double free of bin. While the individual memory
>> leak fixes are fine, both fixes together are problematic.
>>
>> Addresses-Coverity: ("Double free")
>> Fixes: 29cd13cfd762 ("drm/v3d: Fix memory leak in v3d_submit_cl_ioctl")
>> Fixes: 0d352a3a8a1f (" rm/v3d: don't leak bin job if v3d_job_init fails.")
>> Signed-off-by: Colin Ian King
>
> That sounds like wrong merge resolution somewhere, and we don't have those
> patches merged together in any final tree yet anywhere. What's this based
> on?
> -Daniel
linux-next
Colin
>
>> ---
>> drivers/gpu/drm/v3d/v3d_gem.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
>> index 549dde83408b..37515e47b47e 100644
>> --- a/drivers/gpu/drm/v3d/v3d_gem.c
>> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
>> @@ -568,7 +568,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
>> ret = v3d_job_init(v3d, file_priv, &bin->base,
>> v3d_job_free, args->in_sync_bcl);
>> if (ret) {
>> -kfree(bin);
>> v3d_job_put(&render->base);
>> kfree(bin);
>> return ret;
>> --
>> 2.20.1
>>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/v3d: fix double free of bin
On Thu, Oct 24, 2019 at 2:43 PM Colin Ian King wrote:
>
> On 24/10/2019 13:38, Daniel Vetter wrote:
> > On Thu, Oct 24, 2019 at 11:48:01AM +0100, Colin King wrote:
> >> From: Colin Ian King
> >>
> >> Two different fixes have addressed the same memory leak of bin and
> >> this now causes a double free of bin. While the individual memory
> >> leak fixes are fine, both fixes together are problematic.
> >>
> >> Addresses-Coverity: ("Double free")
> >> Fixes: 29cd13cfd762 ("drm/v3d: Fix memory leak in v3d_submit_cl_ioctl")
> >> Fixes: 0d352a3a8a1f (" rm/v3d: don't leak bin job if v3d_job_init fails.")
> >> Signed-off-by: Colin Ian King
> >
> > That sounds like wrong merge resolution somewhere, and we don't have those
> > patches merged together in any final tree yet anywhere. What's this based
> > on?
> > -Daniel
>
> linux-next
Ok adding Stephen. There's a merge conflict between drm-misc-fixes and
drm-next (I think) and the merge double-added the kfree(bin). See
above for the relevant sha1. Dave is already on here as a heads-up,
but also adding drm-misc maintainers.
Cheers, Daniel
>
> Colin
> >
> >> ---
> >> drivers/gpu/drm/v3d/v3d_gem.c | 1 -
> >> 1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
> >> index 549dde83408b..37515e47b47e 100644
> >> --- a/drivers/gpu/drm/v3d/v3d_gem.c
> >> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> >> @@ -568,7 +568,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
> >> ret = v3d_job_init(v3d, file_priv, &bin->base,
> >> v3d_job_free, args->in_sync_bcl);
> >> if (ret) {
> >> -kfree(bin);
> >> v3d_job_put(&render->base);
> >> kfree(bin);
> >> return ret;
> >> --
> >> 2.20.1
> >>
> >
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 5/6] arm64: dts: allwinner: a64: Add MIPI DSI pipeline
On Thu, Oct 17, 2019 at 3:22 PM Maxime Ripard wrote:
>
> On Wed, Oct 16, 2019 at 02:19:44PM +0530, Jagan Teki wrote:
> > On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard wrote:
> > >
> > > On Mon, Oct 14, 2019 at 05:37:50PM +0530, Jagan Teki wrote:
> > > > On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard wrote:
> > > > >
> > > > > On Sat, Oct 05, 2019 at 07:49:12PM +0530, Jagan Teki wrote:
> > > > > > Add MIPI DSI pipeline for Allwinner A64.
> > > > > >
> > > > > > - dsi node, with A64 compatible since it doesn't support
> > > > > > DSI_SCLK gating unlike A33
> > > > > > - dphy node, with A64 compatible with A33 fallback since
> > > > > > DPHY on A64 and A33 is similar
> > > > > > - finally, attach the dsi_in to tcon0 for complete MIPI DSI
> > > > > >
> > > > > > Signed-off-by: Jagan Teki
> > > > > > Tested-by: Merlijn Wajer
> > > > > > ---
> > > > > > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38
> > > > > > +++
> > > > > > 1 file changed, 38 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > index 69128a6dfc46..ad4170b8aee0 100644
> > > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > > > > @@ -382,6 +382,12 @@
> > > > > > #address-cells = <1>;
> > > > > > #size-cells = <0>;
> > > > > > reg = <1>;
> > > > > > +
> > > > > > + tcon0_out_dsi: endpoint@1 {
> > > > > > + reg = <1>;
> > > > > > + remote-endpoint =
> > > > > > <&dsi_in_tcon0>;
> > > > > > +
> > > > > > allwinner,tcon-channel = <1>;
> > > > > > + };
> > > > > > };
> > > > > > };
> > > > > > };
> > > > > > @@ -1003,6 +1009,38 @@
> > > > > > status = "disabled";
> > > > > > };
> > > > > >
> > > > > > + dsi: dsi@1ca {
> > > > > > + compatible = "allwinner,sun50i-a64-mipi-dsi";
> > > > > > + reg = <0x01ca 0x1000>;
> > > > > > + interrupts = ;
> > > > > > + clocks = <&ccu CLK_BUS_MIPI_DSI>;
> > > > > > + clock-names = "bus";
> > > > >
> > > > > This won't validate with the bindings you have either here, since it
> > > > > still expects bus and mod.
> > > > >
> > > > > I guess in that cas, we can just drop clock-names, which will require
> > > > > a bit of work on the driver side as well.
> > > >
> > > > Okay.
> > > > mod clock is not required for a64, ie reason we have has_mod_clk quirk
> > > > patch. Adjust the clock-names: on dt-bindings would make sense here,
> > > > what do you think?
> > >
> > > I'm confused, what are you suggesting?
> >
> > Sorry for the confusion.
> >
> > The mod clock is not required for A64 and we have a patch for handling
> > mod clock using has_mod_clk quirk(on the series), indeed the mod clock
> > is available in A31 and not needed for A64. So, to satisfy this
> > requirement the clock-names on dt-bindings can update to make mod
> > clock-name is optional and bus clock is required.
>
> No, the bus clock name is not needed if there's only one clock.
Looks like we need "bus" clock required since the
devm_regmap_init_mmio_clk is created only if bus clock-names added in
dt.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: extend syncobj query ability v3
在 2019/10/24 4:21, Sean Paul 写道:
> On Tue, Jul 30, 2019 at 9:22 AM Chunming Zhou wrote:
>>
>> 在 2019/7/30 21:17, Koenig, Christian 写道:
>>> Am 30.07.19 um 15:02 schrieb Chunming Zhou:
user space needs a flexiable query ability.
So that umd can get last signaled or submitted point.
v2:
add sanitizer checking.
v3:
rebase
Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
Signed-off-by: Chunming Zhou
Cc: Lionel Landwerlin
Cc: Christian König
Reviewed-by: Lionel Landwerlin
>>> Reviewed-by: Christian König
>>>
>>> Lionel is the Intel code using this already public? Or David any chance
>>> that we can get a public amdvlk release using this?
>> In latest public amdvlk, We should be able to see how timeline syncobj
>> is used in it.
>>
> I couldn't find this anywhere, could you please provide a link?
https://github.com/GPUOpen-Drivers/xgl/blob/dev/icd/api/vk_semaphore.cpp
You can check the source here.
Cheers,
-David
>
> Sean
>
>> -David
>>
>>> Christian.
>>>
---
drivers/gpu/drm/drm_syncobj.c | 37 +--
include/uapi/drm/drm.h| 3 ++-
2 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index cecff2e447b1..d4432f1513ac 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device
*dev, void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
-if (args->pad != 0)
+if (args->flags != 0)
return -EINVAL;
if (args->count_handles == 0)
@@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev,
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
-if (args->pad != 0)
+if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
return -EINVAL;
if (args->count_handles == 0)
@@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device
*dev, void *data,
fence = drm_syncobj_fence_get(syncobjs[i]);
chain = to_dma_fence_chain(fence);
if (chain) {
-struct dma_fence *iter, *last_signaled = NULL;
-
-dma_fence_chain_for_each(iter, fence) {
-if (iter->context != fence->context) {
-dma_fence_put(iter);
-/* It is most likely that timeline has
- * unorder points. */
-break;
+struct dma_fence *iter, *last_signaled =
+dma_fence_get(fence);
+
+if (args->flags &
+DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
+point = fence->seqno;
+} else {
+dma_fence_chain_for_each(iter, fence) {
+if (iter->context != fence->context) {
+dma_fence_put(iter);
+/* It is most likely that
timeline has
+* unorder points. */
+break;
+}
+dma_fence_put(last_signaled);
+last_signaled = dma_fence_get(iter);
}
-dma_fence_put(last_signaled);
-last_signaled = dma_fence_get(iter);
+point = dma_fence_is_signaled(last_signaled) ?
+last_signaled->seqno :
+
to_dma_fence_chain(last_signaled)->prev_seqno;
}
-point = dma_fence_is_signaled(last_signaled) ?
-last_signaled->seqno :
-to_dma_fence_chain(last_signaled)->prev_seqno;
dma_fence_put(last_signaled);
} else {
point = 0;
}
+dma_fence_put(fence);
ret = copy_to_user(&points[i], &point, sizeof(uint64_t));
ret = ret ? -EFAULT : 0;
if (ret)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 661d73f9a919..fd987ce24d9f 1006
Re: [PATCH v3 00/21] drm: Add support for bus-format negotiation
On Thu, 24 Oct 2019 13:27:16 +0200 Neil Armstrong wrote: > hi Boris, > > On 23/10/2019 17:44, Boris Brezillon wrote: > > This patch series aims at adding support for runtime bus-format > > negotiation between all elements of the > > 'encoder -> bridges -> connector/display' section of the pipeline. > > > > In order to support that, we need drm bridges to fully take part in the > > atomic state validation process, which requires adding a > > drm_bridge_state and a new drm_bridge_funcs.atomic_check() hook. > > Once those basic building blocks are in place, we can add new hooks to > > allow bus format negotiation (those are called just before > > ->atomic_check()). The bus format selection is done at runtime by > > testing all possible combinations across the whole bridge chain until > > one is reported to work. > > > > Major changes since v2: > > * Get rid of the dummy bridge embedded in drm_encoder and let encoder > > drivers provide their own bridge element > > * Clarify APIs and improve the doc > > * Propagate bus flags by default > > Seems you forgot my reviewed-bys on patches 5, 8, 11 & 13 Oops, indeed. Can you add them back? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 L.S.S. changed: What|Removed |Added CC||ragnaros39...@yandex.com --- Comment #151 from L.S.S. --- Created attachment 145807 --> https://bugs.freedesktop.org/attachment.cgi?id=145807&action=edit captured GCVM_L2_PROTECTION_FAULT errors in the log. This was captured on 5.4(rc) kernel. I'm having similar issues with Navi on Manjaro (both 5.3 and 5.4 kernels). Both kernels were from official Manjaro repos. It's almost 100% reproducible using Cinnamon's file manager, Nemo. It can happen right after I start it, or after I click something (such as opening a folder). Interestingly, I haven't gotten a freeze from use web browsers (Firefox, Chromium) just yet. When the system froze, the rest of the stuffs are still running. The froze happened in the morning and since I was about to leave for work I left the system as is (until I get back home in the evening). The xmrig (CPU) mining session in the background continued to work as normal as observed from the pool's dashboard. It seems the protection fault errors would appear after the system has frozen long enough (I only saw it appear at the time I left it on frozen for a while, and the rest of the times I reset my system right after it froze). If resetting the system only a short a while after the freeze happened, the log will end only at "ring sdma0 timeout". It seems the "nodma nongg" trick partially worked on 5.3 (5.3.6 to be precise) as the system hasn't frozen for the time being (even when using Nemo). It however, doesn't work with the 5.4 (rc) kernel as I still got a freeze caused by the same "ring sdma0 timeout" error. Off-topic: On 5.3 kernel, the mouse cursor feels sluggish as if my monitor is running at 30Hz (while xrandr reports it's indeed 60Hz), while the mouse cursor works fine on 5.4(rc) kernel. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111762] RX 5700 XT Navi - amdgpu.ppfeaturemask=0xffffffff causes stuttering and does not unlock clock/voltage/power controls
https://bugs.freedesktop.org/show_bug.cgi?id=111762 L.S.S. changed: What|Removed |Added CC||ragnaros39...@yandex.com --- Comment #5 from L.S.S. --- I can also confirm the issue exists. Setting amdgpu.ppfeaturemask=0x doesn't allow me to access the "States Table" section in radeon-profile, as if the parameter was ignored. As for the stutter issue, I don't know what exactly it is as I don't notice any difference with or without the parameter. On 5.3 kernel, the mouse feels sluggish as if my monitor is running at 30Hz, but it's fine on 5.4 (rc) kernel. This is observed on official Manjaro kernels. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 1/3] gpu: host1x: Remove implicit IOMMU backing on client's registration
On Thu, Oct 24, 2019 at 04:35:13PM +0300, Dmitry Osipenko wrote: > 24.10.2019 14:50, Thierry Reding пишет: > > On Sun, Jun 23, 2019 at 08:37:41PM +0300, Dmitry Osipenko wrote: > >> On ARM32 we don't want any of the clients device to be backed by the > >> implicit domain, simply because we can't afford such a waste on older > >> Tegra SoCs that have very few domains available in total. The recent IOMMU > >> support addition for the Video Decoder hardware uncovered the problem > >> that an unfortunate drivers probe order results in the DRM driver probe > >> failure if CONFIG_ARM_DMA_USE_IOMMU=y due to a shortage of IOMMU domains > >> caused by the implicit backing. The host1x_client_register() is a common > >> function that is invoked by all of the relevant DRM drivers during theirs > >> probe and hence it is convenient to remove the implicit backing there, > >> resolving the problem. > >> > >> Signed-off-by: Dmitry Osipenko > >> --- > >> drivers/gpu/host1x/bus.c | 19 +++ > >> 1 file changed, 19 insertions(+) > > > > I don't really want to do this in a central place like this. If we > > really do need this, why can't we do it in the individual drivers? > > Why do you want to duplicate the same action for each driver instead of > doing it in a single common place? I don't mind doing it in a common place in particular, I just don't want to do this within the host1x bus infrastructure. This is really a policy decision that should be up to drivers. Consider the case where we had a different host1x driver (for V4L2 for example) that would actually want to use the DMA API. In that case we may want to detach in the DRM driver but not the V4L2 driver. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/3] drm/tegra: Fix 2d and 3d clients detaching from IOMMU domain
On Thu, Oct 24, 2019 at 04:28:41PM +0300, Dmitry Osipenko wrote:
> 24.10.2019 14:58, Thierry Reding пишет:
> > On Sun, Jun 23, 2019 at 08:37:42PM +0300, Dmitry Osipenko wrote:
> >> This should should fire up on the DRM's driver module re-loader because
> >> there won't be enough available domains on older Tegra SoCs.
> >>
> >> Cc: stable
> >> Fixes: 0c407de5ed1a ("drm/tegra: Refactor IOMMU attach/detach")
> >> Signed-off-by: Dmitry Osipenko
> >> ---
> >> drivers/gpu/drm/tegra/dc.c | 4 ++--
> >> drivers/gpu/drm/tegra/drm.c | 9 ++---
> >> drivers/gpu/drm/tegra/drm.h | 3 ++-
> >> drivers/gpu/drm/tegra/gr2d.c | 4 ++--
> >> drivers/gpu/drm/tegra/gr3d.c | 4 ++--
> >> 5 files changed, 14 insertions(+), 10 deletions(-)
> >
> > I think I understand what this is trying to do, but the commit message
> > does not help at all. So what's really going on here is that we need to
> > detach the device from the group regardless of whether we're sharing the
> > group or not, just like we attach groups to the shared domain whether
> > they share the same group or not.
>
> Yes, the commit's message could be improved.
>
> > But in that case, I wonder if it's even worth splitting groups the way
> > we are right now. Wouldn't it be better to just put all the devices into
> > the same group and be done with it?
> >
> > The current code gives me headaches every time I read it, so if we can
> > just make it so that all the devices under the DRM device share the same
> > group, this would become a lot easier to deal with. I'm not really
> > convinced that it makes much sense to keep them on separate domains,
> > especially given the constraints on the number of domains available on
> > earlier Tegra devices.
> >
> > Note that sharing a group will also make it much easier for these to use
> > the DMA API if it is backed by an IOMMU.
>
> Probably I'm blanking on everything about IOMMU now.. could you please
> remind me what "IOMMU group" is?
>
> Isn't it that each IOMMU group relates to the HW ID (SWGROUP)? But then
> each display controller has its own SWGROUP.. and thus that sharing just
> doesn't make any sense, hm.
IOMMU groups are not directly related to SWGROUPs. But by default the
IOMMU framework will share a domain between members of the same IOMMU
group. Seems like that's really what we want here, so that when we do
use the DMA API, all the devices part of the DRM device get attached to
the same IOMMU domain, yet if we don't want to use the DMA API we only
need to detach the one group from the backing.
Thierry
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/dp: Add function to parse EDID descriptors for adaptive sync limits
On Thu, Oct 24, 2019 at 02:34:00PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 24, 2019 at 12:31:06PM +0200, Thierry Reding wrote:
> > On Wed, Oct 23, 2019 at 05:00:41PM -0700, Manasi Navare wrote:
> > > Adaptive Sync is a VESA feature so add a DRM core helper to parse
> > > the EDID's detailed descritors to obtain the adaptive sync monitor range.
> > > Store this info as part fo drm_display_info so it can be used
> > > across all drivers.
> > > This part of the code is stripped out of amdgpu's function
> > > amdgpu_dm_update_freesync_caps() to make it generic and be used
> > > across all DRM drivers
> > >
> > > Cc: Ville Syrjälä
> > > Cc: Harry Wentland
> > > Cc: Clinton A Taylor
> > > Signed-off-by: Manasi Navare
> > > ---
> > > drivers/gpu/drm/drm_edid.c | 49 +
> > > include/drm/drm_connector.h | 25 +++
> > > include/drm/drm_edid.h | 2 ++
> > > 3 files changed, 76 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index 474ac04d5600..97dd1200773e 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -4707,6 +4707,52 @@ static void drm_parse_cea_ext(struct drm_connector
> > > *connector,
> > > }
> > > }
> > >
> > > +void drm_get_adaptive_sync_limits(struct drm_connector *connector,
> > > + const struct edid *edid)
> > > +{
> > > + struct drm_display_info *info = &connector->display_info;
> > > + const struct detailed_timing *timing;
> > > + const struct detailed_non_pixel *data;
> > > + const struct detailed_data_monitor_range *range;
> > > + int i;
> >
> > This can be unsigned int.
>
> Please no. A loop iterator called 'i' should always be a normal signed int.
What? Where's that rule written down? In my experience it's always
better to use as restrictive a type as possible. It's really annoying
when GCC suddenly starts complaining about comparison between signed and
unsigned. So if a variable can never contain a signed value, why risk
the ambiguity? The value goes from 0 to 4, the sign bit is useless.
> > > + /*
> > > + * Restrict Adaptive Sync only for dp and edp
> > > + */
> > > + if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
> > > + connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> > > + return;
> > > +
> > > + if (edid->version <= 1 && !(edid->version == 1 && edid->revision > 1))
> > > + return;
> > > +
> > > + for (i = 0; i < 4; i++) {
> > > + timing = &edid->detailed_timings[i];
> > > + data= &timing->data.other_data;
> > > + range = &data->data.range;
> > > + /*
> > > + * Check if monitor has continuous frequency mode
> > > + */
> > > + if (data->type != EDID_DETAIL_MONITOR_RANGE)
> > > + continue;
> > > + /*
> > > + * Check for flag range limits only. If flag == 1 then
> > > + * no additional timing information provided.
> > > + * Default GTF, GTF Secondary curve and CVT are not
> > > + * supported
> > > + */
> > > + if (range->flags != 1)
> > > + continue;
> > > +
> > > + info->adaptive_sync.min_vfreq = range->min_vfreq;
> > > + info->adaptive_sync.max_vfreq = range->max_vfreq;
> > > + info->adaptive_sync.pixel_clock_mhz =
> > > + range->pixel_clock_mhz * 10;
> > > + break;
> > > + }
> > > +}
> > > +EXPORT_SYMBOL(drm_get_adaptive_sync_limits);
> > > +
> > > /* A connector has no EDID information, so we've got no EDID to compute
> > > quirks from. Reset
> > > * all of the values which would have been set from EDID
> > > */
> > > @@ -4728,6 +4774,7 @@ drm_reset_display_info(struct drm_connector
> > > *connector)
> > > memset(&info->hdmi, 0, sizeof(info->hdmi));
> > >
> > > info->non_desktop = 0;
> > > + memset(&info->adaptive_sync, 0, sizeof(info->adaptive_sync));
> > > }
> > >
> > > u32 drm_add_display_info(struct drm_connector *connector, const struct
> > > edid *edid)
> > > @@ -4743,6 +4790,8 @@ u32 drm_add_display_info(struct drm_connector
> > > *connector, const struct edid *edi
> > >
> > > info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
> > >
> > > + drm_get_adaptive_sync_limits(connector, edid);
> > > +
> > > DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
> > >
> > > if (edid->revision < 3)
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index 5f8c3389d46f..a27a84270d8d 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -254,6 +254,26 @@ enum drm_panel_orientation {
> > > DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> > > };
> > >
> > > +/**
> > > + * struct drm_adaptive_sync_info - Panel's Adaptive Sync capabilities for
> > > + * &drm_display_info
> > > + *
> > > + * This struct is used to store a Panel's Adaptive Sync ca
[Bug 205277] [amd powerplay] vega10: soc voltage for power state 7 is not changed by overdrive.
https://bugzilla.kernel.org/show_bug.cgi?id=205277 Pelle van Gils (pe...@vangils.xyz) changed: What|Removed |Added Attachment #285585|0 |1 is obsolete|| -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/4] drm/vram: Provide helpers for prepare_fb() and cleanup_fb()
Hi Am 24.10.19 um 14:37 schrieb Daniel Vetter: > On Thu, Oct 24, 2019 at 10:14:00AM +0200, Thomas Zimmermann wrote: >> The implementation of the plane's call-back functions prepare_fb() and >> cleanup_fb() for GEM VRAM helpers are sharable among drivers. >> >> Patch #3 also fixes two bugs that have been present in hibmc since it was >> first added. The primary plane's atomic_update() is not responsible for >> pinning BOs. And it never unpinned unused BOs. VRAM is being exausted >> over time. >> >> The new helpers have been tested to work. >> >> v2: >> * provide helpers for struct drm_simple_display_pipe_funcs, and... >> * ...use them in bochs > > Oh I thought we agreed on changing the simple_pipe type for > prepare/cleanup_fb ... But this works too ofc. On the series: Well, I'm still no fan of the current simple pipe helpers. But after you changed the signature of mode_valid() and explained the reasons, I thought it was more important to have consistent interfaces. > > Reviewed-by: Daniel Vetter Thanks! Best regards Thomas > >> >> Thomas Zimmermann (4): >> drm/vram-helpers: Add helpers for prepare_fb() and cleanup_fb() >> drm/bochs: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers >> drm/hisilicon/hibmc: Use GEM VRAM's prepare_fb() and cleanup_fb() >> helpers >> drm/vboxvideo: Replace prepare_fb()/cleanup_fb() with GEM VRAM helpers >> >> drivers/gpu/drm/bochs/bochs_kms.c | 26 +--- >> drivers/gpu/drm/drm_gem_vram_helper.c | 126 ++ >> .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 14 +- >> drivers/gpu/drm/vboxvideo/vbox_mode.c | 61 + >> include/drm/drm_gem_vram_helper.h | 25 >> 5 files changed, 161 insertions(+), 91 deletions(-) >> >> -- >> 2.23.0 >> > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/dp: Add function to parse EDID descriptors for adaptive sync limits
On Thu, Oct 24, 2019 at 03:54:41PM +0200, Thierry Reding wrote:
> On Thu, Oct 24, 2019 at 02:34:00PM +0300, Ville Syrjälä wrote:
> > On Thu, Oct 24, 2019 at 12:31:06PM +0200, Thierry Reding wrote:
> > > On Wed, Oct 23, 2019 at 05:00:41PM -0700, Manasi Navare wrote:
> > > > Adaptive Sync is a VESA feature so add a DRM core helper to parse
> > > > the EDID's detailed descritors to obtain the adaptive sync monitor
> > > > range.
> > > > Store this info as part fo drm_display_info so it can be used
> > > > across all drivers.
> > > > This part of the code is stripped out of amdgpu's function
> > > > amdgpu_dm_update_freesync_caps() to make it generic and be used
> > > > across all DRM drivers
> > > >
> > > > Cc: Ville Syrjälä
> > > > Cc: Harry Wentland
> > > > Cc: Clinton A Taylor
> > > > Signed-off-by: Manasi Navare
> > > > ---
> > > > drivers/gpu/drm/drm_edid.c | 49 +
> > > > include/drm/drm_connector.h | 25 +++
> > > > include/drm/drm_edid.h | 2 ++
> > > > 3 files changed, 76 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > index 474ac04d5600..97dd1200773e 100644
> > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > @@ -4707,6 +4707,52 @@ static void drm_parse_cea_ext(struct
> > > > drm_connector *connector,
> > > > }
> > > > }
> > > >
> > > > +void drm_get_adaptive_sync_limits(struct drm_connector *connector,
> > > > + const struct edid *edid)
> > > > +{
> > > > + struct drm_display_info *info = &connector->display_info;
> > > > + const struct detailed_timing *timing;
> > > > + const struct detailed_non_pixel *data;
> > > > + const struct detailed_data_monitor_range *range;
> > > > + int i;
> > >
> > > This can be unsigned int.
> >
> > Please no. A loop iterator called 'i' should always be a normal signed int.
>
> What? Where's that rule written down? In my experience it's always
> better to use as restrictive a type as possible. It's really annoying
> when GCC suddenly starts complaining about comparison between signed and
> unsigned. So if a variable can never contain a signed value, why risk
> the ambiguity? The value goes from 0 to 4, the sign bit is useless.
Dunno if it's really written down anywhere. It's just something
experience has taught. IIRC there's also a rant from Linus about this
somewhere. Hm, can't find that one right now, but Andrew Morton also
seems to agree: https://lwn.net/Articles/309279/
Ah, here is one Linus rant about unsigned array indexes:
https://yarchive.net/comp/linux/gcc.html
My opinion: unsigned is an very *dangerous* keyword in C that demands
your respect. You should never use it without thinking first what the
ramifications are. You always have to have the promotion rules clear
in you mind when you do any kind of arithmetic with >= unsigned int
type. And common idioms such as 'int i' should be respected so as to
not cause unexpected hair loss to other developers when they decide
to make the loop iterate backwards.
>
> > > > + /*
> > > > +* Restrict Adaptive Sync only for dp and edp
> > > > +*/
> > > > + if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort
> > > > &&
> > > > + connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> > > > + return;
> > > > +
> > > > + if (edid->version <= 1 && !(edid->version == 1 &&
> > > > edid->revision > 1))
> > > > + return;
> > > > +
> > > > + for (i = 0; i < 4; i++) {
> > > > + timing = &edid->detailed_timings[i];
> > > > + data= &timing->data.other_data;
> > > > + range = &data->data.range;
> > > > + /*
> > > > +* Check if monitor has continuous frequency mode
> > > > +*/
> > > > + if (data->type != EDID_DETAIL_MONITOR_RANGE)
> > > > + continue;
> > > > + /*
> > > > +* Check for flag range limits only. If flag == 1 then
> > > > +* no additional timing information provided.
> > > > +* Default GTF, GTF Secondary curve and CVT are not
> > > > +* supported
> > > > +*/
> > > > + if (range->flags != 1)
> > > > + continue;
> > > > +
> > > > + info->adaptive_sync.min_vfreq = range->min_vfreq;
> > > > + info->adaptive_sync.max_vfreq = range->max_vfreq;
> > > > + info->adaptive_sync.pixel_clock_mhz =
> > > > + range->pixel_clock_mhz * 10;
> > > > + break;
> > > > + }
> > > > +}
> > > > +EXPORT_SYMBOL(drm_get_adaptive_sync_limits);
> > > > +
> > > > /* A connector has no EDID information, so we've got no EDID to
> > > > compute quirks from. Reset
> > > > * all of the value
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #152 from L.S.S. --- UPDATE: I just got another freeze on 5.3.6 kernel. The same GCVM_L2_PROTECTION_FAULT error followed by a ring sdma0 timeout. So it seems AMD_DEBUG="nodma nongg" doesn't really work for me. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Question regarding "reserved-memory"
Hi Folks,
I have a question regarding "reserved-memory". I am using an Arm Juno
platform which has a chunk of ram in its fpga. I intend to make this
memory as reserved so that it can be shared between various devices
for passing framebuffer.
My dts looks like the following:-
/ {
// some nodes
tlx@6000 {
compatible = "simple-bus";
...
juno_wrapper {
... /* here we have all the nodes */
/* corresponding to the devices in the fpga */
memory@d00 {
device_type = "memory";
reg = <0x00 0x6000 0x00 0x800>;
};
reserved-memory {
#address-cells = <0x01>;
#size-cells = <0x01>;
ranges;
framebuffer@d00 {
compatible = "shared-dma-pool";
linux,cma-default;
reusable;
reg = <0x00 0x6000 0x00 0x800>;
phandle = <0x44>;
};
};
...
}
}
...
}
Note that the depth of the "reserved-memory" node is 3.
Refer __fdt_scan_reserved_mem() :-
if (!found && depth == 1 && strcmp(uname, "reserved-memory") == 0) {
if (__reserved_mem_check_root(node) != 0) {
pr_err("Reserved memory: unsupported node
format, ignoring\n");
/* break scan */
return 1;
}
found = 1;
/* scan next node */
return 0;
}
It expects the "reserved-memory" node to be at depth == 1 and so it
does not probe it in our case.
Niether from the
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
nor from commit - e8d9d1f5485b52ec3c4d7af839e6914438f6c285,
I could understand the reason for such restriction.
So, I seek the community's advice as to whether I should fix up
__fdt_scan_reserved_mem() so as to do away with the restriction or
put the "reserved-memory" node outside of 'tlx@6000' (which looks
logically incorrect as the memory is on the fpga platform).
Thanks,
Ayan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/prime: Fix mmap fake offset for drm_gem_object_funcs.mmap
On Thu, Oct 24, 2019 at 7:32 AM Daniel Vetter wrote:
>
> On Thu, Oct 24, 2019 at 11:02:40AM +0200, Gerd Hoffmann wrote:
> > On Wed, Oct 23, 2019 at 05:22:26PM -0500, Rob Herring wrote:
> > > Commit c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs")
> > > introduced a GEM object mmap() hook which is expected to subtract the
> > > fake offset from vm_pgoff.
> >
> > Long-term it is probably a good idea to just remove the fake offset
> > handling from drivers. But that'll only work once all drivers switched
> > away from custom fops->mmap handlers so we can handle the offset -> obj
> > lookup in the drm core for everybody.
> >
> > So let's go this way for now.
> >
> > Acked-by: Gerd Hoffmann
>
> Uh this sounds like doubling down on rather horrible semantics. Can we at
> least stop the mess instead of baking it in for real? The hook is very
> very new after all. I.e.
> - Document that obj->funcs->mmap will have 0 offset in the kerneldoc.
> - Remove the subtracting from the shmem helper
> - In ttm_bo_mmap_obj re-add the offset with a huge FIXME comment.
> - Adjust drm_gem_mmap_obj to do that same for obj->funcs->mmap and also
> document the expectation there too.
Okay.
> This feels like very much going the wrong direction ...
>
> Also I guess Gerd didn't really test this prime mmap support?
Perhaps because at least parts of the IGT "vgem" tests really have
nothing specific for "vgem" and there doesn't seem to be another test
case that does run doing the same thing. And none of the IGT prime
tests run without an Intel driver. Looking at IGT always makes me sad,
and then I move on to other things...
BTW, are there IGT test results for vgem/vkms somewhere? I didn't have
any luck finding anything.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/dp: Add function to parse EDID descriptors for adaptive sync limits
On Thu, Oct 24, 2019 at 05:20:32PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 24, 2019 at 03:54:41PM +0200, Thierry Reding wrote:
> > On Thu, Oct 24, 2019 at 02:34:00PM +0300, Ville Syrjälä wrote:
> > > On Thu, Oct 24, 2019 at 12:31:06PM +0200, Thierry Reding wrote:
> > > > On Wed, Oct 23, 2019 at 05:00:41PM -0700, Manasi Navare wrote:
> > > > > Adaptive Sync is a VESA feature so add a DRM core helper to parse
> > > > > the EDID's detailed descritors to obtain the adaptive sync monitor
> > > > > range.
> > > > > Store this info as part fo drm_display_info so it can be used
> > > > > across all drivers.
> > > > > This part of the code is stripped out of amdgpu's function
> > > > > amdgpu_dm_update_freesync_caps() to make it generic and be used
> > > > > across all DRM drivers
> > > > >
> > > > > Cc: Ville Syrjälä
> > > > > Cc: Harry Wentland
> > > > > Cc: Clinton A Taylor
> > > > > Signed-off-by: Manasi Navare
> > > > > ---
> > > > > drivers/gpu/drm/drm_edid.c | 49
> > > > > +
> > > > > include/drm/drm_connector.h | 25 +++
> > > > > include/drm/drm_edid.h | 2 ++
> > > > > 3 files changed, 76 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > > index 474ac04d5600..97dd1200773e 100644
> > > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > > @@ -4707,6 +4707,52 @@ static void drm_parse_cea_ext(struct
> > > > > drm_connector *connector,
> > > > > }
> > > > > }
> > > > >
> > > > > +void drm_get_adaptive_sync_limits(struct drm_connector *connector,
> > > > > + const struct edid *edid)
> > > > > +{
> > > > > + struct drm_display_info *info = &connector->display_info;
> > > > > + const struct detailed_timing *timing;
> > > > > + const struct detailed_non_pixel *data;
> > > > > + const struct detailed_data_monitor_range *range;
> > > > > + int i;
> > > >
> > > > This can be unsigned int.
> > >
> > > Please no. A loop iterator called 'i' should always be a normal signed
> > > int.
> >
> > What? Where's that rule written down? In my experience it's always
> > better to use as restrictive a type as possible. It's really annoying
> > when GCC suddenly starts complaining about comparison between signed and
> > unsigned. So if a variable can never contain a signed value, why risk
> > the ambiguity? The value goes from 0 to 4, the sign bit is useless.
>
> Dunno if it's really written down anywhere. It's just something
> experience has taught. IIRC there's also a rant from Linus about this
> somewhere. Hm, can't find that one right now, but Andrew Morton also
> seems to agree: https://lwn.net/Articles/309279/
> Ah, here is one Linus rant about unsigned array indexes:
> https://yarchive.net/comp/linux/gcc.html
It's interesting that none of those actually give a real reason why
unsigned int shouldn't be used for variables called i.
> My opinion: unsigned is an very *dangerous* keyword in C that demands
> your respect. You should never use it without thinking first what the
> ramifications are. You always have to have the promotion rules clear
> in you mind when you do any kind of arithmetic with >= unsigned int
> type. And common idioms such as 'int i' should be respected so as to
> not cause unexpected hair loss to other developers when they decide
> to make the loop iterate backwards.
I would argue that when you do things like make a loop iterate backwards
you better know what variable types you're dealing with.
Anyway, this is clearly very subjective, so feel free to let this be int
if you prefer.
> > > > > + /*
> > > > > + * Restrict Adaptive Sync only for dp and edp
> > > > > + */
> > > > > + if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort
> > > > > &&
> > > > > + connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> > > > > + return;
> > > > > +
> > > > > + if (edid->version <= 1 && !(edid->version == 1 &&
> > > > > edid->revision > 1))
> > > > > + return;
> > > > > +
> > > > > + for (i = 0; i < 4; i++) {
> > > > > + timing = &edid->detailed_timings[i];
> > > > > + data= &timing->data.other_data;
> > > > > + range = &data->data.range;
> > > > > + /*
> > > > > + * Check if monitor has continuous frequency mode
> > > > > + */
> > > > > + if (data->type != EDID_DETAIL_MONITOR_RANGE)
> > > > > + continue;
> > > > > + /*
> > > > > + * Check for flag range limits only. If flag == 1 then
> > > > > + * no additional timing information provided.
> > > > > + * Default GTF, GTF Secondary curve and CVT are not
> > > > > + * supported
> > > > > + */
> > > > > + if (range->flags != 1)
> > > > > + continue
[PATCH 1/5] drm/udl: Clear BO vmapping pointer after unmapping BO memory
Unmapping the BO memory with udl_gem_vunmap() creates a dangling pointer in struct udl_gem_object.vmapping. This can crash udl_handle_damage(), which check the pointer's value for NULL. Clear the pointer to NULL and let udl_handle_damage() re-establish the mapping if necessary. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/udl/udl_gem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c index b23a5c2fcd80..3ea0cd9ae2d6 100644 --- a/drivers/gpu/drm/udl/udl_gem.c +++ b/drivers/gpu/drm/udl/udl_gem.c @@ -174,6 +174,7 @@ void udl_gem_vunmap(struct udl_gem_object *obj) } vunmap(obj->vmapping); + obj->vmapping = NULL; udl_gem_put_pages(obj); } -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/5] drm/udl: Convert to generic fbdev emulation
This patchset replaces udl's fbdev code with the generic implementation. The first patch fixes a bug that didn't trigger yet because the current fbdev never unmaps the BO. Patches 2 to 3 add missing interfaces to the udl driver. Patch 4 sets mapping flags. In the final patch, we remove a lot of code and set a few helpers instead. The patchset was tested by running the fbdev console, X11, and Weston on a DisplayLink adapter. Thomas Zimmermann (5): drm/udl: Clear BO vmapping pointer after unmapping BO memory drm/udl: Set drm_driver.gem_prime_mmap drm/udl: Add GEM object functions for free(), vmap(), and vunmap() drm/udl: Map BO memory pages in unencrypted mode drm/udl: Replace fbdev code with generic emulation drivers/gpu/drm/udl/udl_drv.c | 4 + drivers/gpu/drm/udl/udl_drv.h | 4 - drivers/gpu/drm/udl/udl_fb.c | 263 +- drivers/gpu/drm/udl/udl_gem.c | 40 - drivers/gpu/drm/udl/udl_main.c| 2 - drivers/gpu/drm/udl/udl_modeset.c | 3 +- 6 files changed, 48 insertions(+), 268 deletions(-) -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm/udl: Add GEM object functions for free(), vmap(), and vunmap()
Implementing vmap() and vunmap() of struct drm_gem_object_funcs is
required by generic fbdev emulation. Supporting free() is easy as
well. More udl-specific interfaces can probably be replaced by GEM
object functions in later patches.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/udl/udl_gem.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
index 3ea0cd9ae2d6..6ca097c270d6 100644
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ b/drivers/gpu/drm/udl/udl_gem.c
@@ -11,6 +11,8 @@
#include "udl_drv.h"
+static const struct drm_gem_object_funcs udl_gem_object_funcs;
+
struct udl_gem_object *udl_gem_alloc_object(struct drm_device *dev,
size_t size)
{
@@ -25,6 +27,7 @@ struct udl_gem_object *udl_gem_alloc_object(struct drm_device
*dev,
return NULL;
}
+ obj->base.funcs = &udl_gem_object_funcs;
obj->flags = UDL_BO_CACHEABLE;
return obj;
}
@@ -230,3 +233,34 @@ int udl_gem_mmap(struct drm_file *file, struct drm_device
*dev,
mutex_unlock(&udl->gem_lock);
return ret;
}
+
+/*
+ * Helpers for struct drm_gem_object_funcs
+ */
+
+static void udl_gem_object_free(struct drm_gem_object *obj)
+{
+ udl_gem_free_object(obj);
+}
+
+static void *udl_gem_object_vmap(struct drm_gem_object *obj)
+{
+ struct udl_gem_object *uobj = to_udl_bo(obj);
+ int ret;
+
+ ret = udl_gem_vmap(uobj);
+ if (ret)
+ return ERR_PTR(ret);
+ return uobj->vmapping;
+}
+
+static void udl_gem_object_vunmap(struct drm_gem_object *obj, void *vaddr)
+{
+ return udl_gem_vunmap(to_udl_bo(obj));
+}
+
+static const struct drm_gem_object_funcs udl_gem_object_funcs = {
+ .free = udl_gem_object_free,
+ .vmap = udl_gem_object_vmap,
+ .vunmap = udl_gem_object_vunmap,
+};
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/udl: Map BO memory pages in unencrypted mode
The udl driver's fbdev code maps pages in unencrypted mode. To switch over to generic fbdev emulation, we set this flag in the BO mapping code. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/udl/udl_gem.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c index 6ca097c270d6..68e4757e83f5 100644 --- a/drivers/gpu/drm/udl/udl_gem.c +++ b/drivers/gpu/drm/udl/udl_gem.c @@ -100,6 +100,9 @@ int udl_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) update_vm_cache_attr(to_udl_bo(vma->vm_private_data), vma); + /* We don't want the framebuffer to be mapped encrypted */ + vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); + return ret; } @@ -158,7 +161,7 @@ int udl_gem_vmap(struct udl_gem_object *obj) return -ENOMEM; return 0; } - + ret = udl_gem_get_pages(obj); if (ret) return ret; -- 2.23.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm/udl: Set drm_driver.gem_prime_mmap
This interface is required by the mmap support of generic
fbdev emulation.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/udl/udl_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 8426669433e4..15ad7a338f9d 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -74,6 +74,7 @@ static struct drm_driver driver = {
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_export = udl_gem_prime_export,
.gem_prime_import = udl_gem_prime_import,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/udl: Replace fbdev code with generic emulation
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/udl/udl_drv.c | 3 +
drivers/gpu/drm/udl/udl_drv.h | 4 -
drivers/gpu/drm/udl/udl_fb.c | 263 +-
drivers/gpu/drm/udl/udl_main.c| 2 -
drivers/gpu/drm/udl/udl_modeset.c | 3 +-
5 files changed, 8 insertions(+), 267 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 15ad7a338f9d..6beaa1109c2c 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -7,6 +7,7 @@
#include
#include
+#include
#include
#include
#include
@@ -62,6 +63,8 @@ static struct drm_driver driver = {
.driver_features = DRIVER_MODESET | DRIVER_GEM,
.release = udl_driver_release,
+ .lastclose = drm_fb_helper_lastclose,
+
/* gem hooks */
.gem_free_object_unlocked = udl_gem_free_object,
.gem_vm_ops = &udl_gem_vm_ops,
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 12a970fd9a87..5f8a7ac084f6 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -50,8 +50,6 @@ struct urb_list {
size_t size;
};
-struct udl_fbdev;
-
struct udl_device {
struct drm_device drm;
struct device *dev;
@@ -65,7 +63,6 @@ struct udl_device {
struct urb_list urbs;
atomic_t lost_pixels; /* 1 = a render op failed. Need screen refresh */
- struct udl_fbdev *fbdev;
char mode_buf[1024];
uint32_t mode_buf_len;
atomic_t bytes_rendered; /* raw pixel-bytes driver asked to render */
@@ -111,7 +108,6 @@ int udl_init(struct udl_device *udl);
void udl_fini(struct drm_device *dev);
int udl_fbdev_init(struct drm_device *dev);
-void udl_fbdev_cleanup(struct drm_device *dev);
void udl_fbdev_unplug(struct drm_device *dev);
struct drm_framebuffer *
udl_fb_user_fb_create(struct drm_device *dev,
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index ef3504d06343..43a1da3a56c3 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -19,19 +19,9 @@
#include "udl_drv.h"
-#define DL_DEFIO_WRITE_DELAY(HZ/20) /* fb_deferred_io.delay in jiffies */
-
-static int fb_defio = 0; /* Optionally enable experimental fb_defio mmap
support */
static int fb_bpp = 16;
module_param(fb_bpp, int, S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP);
-module_param(fb_defio, int, S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP);
-
-struct udl_fbdev {
- struct drm_fb_helper helper; /* must be first */
- struct udl_framebuffer ufb;
- int fb_count;
-};
#define DL_ALIGN_UP(x, a) ALIGN(x, a)
#define DL_ALIGN_DOWN(x, a) ALIGN_DOWN(x, a)
@@ -157,123 +147,6 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x,
int y,
return 0;
}
-static int udl_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
-{
- unsigned long start = vma->vm_start;
- unsigned long size = vma->vm_end - vma->vm_start;
- unsigned long offset;
- unsigned long page, pos;
-
- if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT))
- return -EINVAL;
-
- offset = vma->vm_pgoff << PAGE_SHIFT;
-
- if (offset > info->fix.smem_len || size > info->fix.smem_len - offset)
- return -EINVAL;
-
- pos = (unsigned long)info->fix.smem_start + offset;
-
- pr_debug("mmap() framebuffer addr:%lu size:%lu\n",
- pos, size);
-
- /* We don't want the framebuffer to be mapped encrypted */
- vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
-
- while (size > 0) {
- page = vmalloc_to_pfn((void *)pos);
- if (remap_pfn_range(vma, start, page, PAGE_SIZE, PAGE_SHARED))
- return -EAGAIN;
-
- start += PAGE_SIZE;
- pos += PAGE_SIZE;
- if (size > PAGE_SIZE)
- size -= PAGE_SIZE;
- else
- size = 0;
- }
-
- /* VM_IO | VM_DONTEXPAND | VM_DONTDUMP are set by remap_pfn_range() */
- return 0;
-}
-
-/*
- * It's common for several clients to have framebuffer open simultaneously.
- * e.g. both fbcon and X. Makes things interesting.
- * Assumes caller is holding info->lock (for open and release at least)
- */
-static int udl_fb_open(struct fb_info *info, int user)
-{
- struct udl_fbdev *ufbdev = info->par;
- struct drm_device *dev = ufbdev->ufb.base.dev;
- struct udl_device *udl = to_udl(dev);
-
- /* If the USB device is gone, we don't accept new opens */
- if (drm_dev_is_unplugged(&udl->drm))
- return -ENODEV;
-
- ufbdev->fb_count++;
-
-#ifdef CONFIG_DRM_FBDEV_EMULATION
- if (fb_defio && (info->fbdefio == NULL)) {
- /* enable defio at last moment if not disabled by client */
-
- struct fb_deferred_io *fbdefio;
-
- fbdefio = kzalloc(sizeof(struct fb_deferred_io), GFP_KERNEL);
-
-
Re: Question regarding "reserved-memory"
On Thu, Oct 24, 2019 at 9:22 AM Ayan Halder wrote:
>
>
> Hi Folks,
>
> I have a question regarding "reserved-memory". I am using an Arm Juno
> platform which has a chunk of ram in its fpga. I intend to make this
> memory as reserved so that it can be shared between various devices
> for passing framebuffer.
>
> My dts looks like the following:-
>
> / {
> // some nodes
>
> tlx@6000 {
> compatible = "simple-bus";
> ...
>
> juno_wrapper {
>
> ... /* here we have all the nodes */
> /* corresponding to the devices in the fpga */
>
> memory@d00 {
>device_type = "memory";
>reg = <0x00 0x6000 0x00 0x800>;
> };
>
> reserved-memory {
>#address-cells = <0x01>;
>#size-cells = <0x01>;
>ranges;
>
>framebuffer@d00 {
> compatible = "shared-dma-pool";
> linux,cma-default;
> reusable;
> reg = <0x00 0x6000 0x00
> 0x800>;
> phandle = <0x44>;
> };
> };
> ...
> }
> }
> ...
> }
>
> Note that the depth of the "reserved-memory" node is 3.
>
> Refer __fdt_scan_reserved_mem() :-
>
> if (!found && depth == 1 && strcmp(uname, "reserved-memory") == 0) {
>
> if (__reserved_mem_check_root(node) != 0) {
> pr_err("Reserved memory: unsupported node
> format, ignoring\n");
> /* break scan */
> return 1;
> }
> found = 1;
>
> /* scan next node */
> return 0;
> }
>
> It expects the "reserved-memory" node to be at depth == 1 and so it
> does not probe it in our case.
>
> Niether from the
> Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> nor from commit - e8d9d1f5485b52ec3c4d7af839e6914438f6c285,
> I could understand the reason for such restriction.
>
> So, I seek the community's advice as to whether I should fix up
> __fdt_scan_reserved_mem() so as to do away with the restriction or
> put the "reserved-memory" node outside of 'tlx@6000' (which looks
> logically incorrect as the memory is on the fpga platform).
For now, I'm going to say it must be at the root level. I'd guess the
memory@d00 node is also just ignored (wrong unit-address BTW).
I think you're also forgetting the later unflattened parsing of
/reserved-memory. The other complication IIRC is booting with UEFI
does it's own reserved memory table which often uses the DT table as
its source.
Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205277] [amd powerplay] vega10: soc voltage for power state 7 is not changed by overdrive.
https://bugzilla.kernel.org/show_bug.cgi?id=205277 --- Comment #5 from har...@gmx.de --- In the (now obsolete) proposed code, the variable 'i' will become 8, when the for-loop is done. The following if-statement will access something outside the array memory. Something like this may work without problems, but it can trigger a new problem too. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/prime: Fix mmap fake offset for drm_gem_object_funcs.mmap
On Thu, Oct 24, 2019 at 4:39 PM Rob Herring wrote:
>
> On Thu, Oct 24, 2019 at 7:32 AM Daniel Vetter wrote:
> >
> > On Thu, Oct 24, 2019 at 11:02:40AM +0200, Gerd Hoffmann wrote:
> > > On Wed, Oct 23, 2019 at 05:22:26PM -0500, Rob Herring wrote:
> > > > Commit c40069cb7bd6 ("drm: add mmap() to drm_gem_object_funcs")
> > > > introduced a GEM object mmap() hook which is expected to subtract the
> > > > fake offset from vm_pgoff.
> > >
> > > Long-term it is probably a good idea to just remove the fake offset
> > > handling from drivers. But that'll only work once all drivers switched
> > > away from custom fops->mmap handlers so we can handle the offset -> obj
> > > lookup in the drm core for everybody.
> > >
> > > So let's go this way for now.
> > >
> > > Acked-by: Gerd Hoffmann
> >
> > Uh this sounds like doubling down on rather horrible semantics. Can we at
> > least stop the mess instead of baking it in for real? The hook is very
> > very new after all. I.e.
> > - Document that obj->funcs->mmap will have 0 offset in the kerneldoc.
> > - Remove the subtracting from the shmem helper
> > - In ttm_bo_mmap_obj re-add the offset with a huge FIXME comment.
> > - Adjust drm_gem_mmap_obj to do that same for obj->funcs->mmap and also
> > document the expectation there too.
>
> Okay.
>
> > This feels like very much going the wrong direction ...
> >
> > Also I guess Gerd didn't really test this prime mmap support?
>
> Perhaps because at least parts of the IGT "vgem" tests really have
> nothing specific for "vgem" and there doesn't seem to be another test
> case that does run doing the same thing. And none of the IGT prime
> tests run without an Intel driver. Looking at IGT always makes me sad,
> and then I move on to other things...
The only prime test that could be made generic is the kms one, which
still requires crc to make sure the stuff actually works. Everything
else is kinda device specific (and vgem is just the test vehicle to
make writing a testcase possible without actually needing 2 gpus).
Maybe we could do a dumb buffer export, then mmap test, but that
doesn't exist. I think for the prime+kms one there's actually patches
floating around to make it generic.
For the others I really don't think you can make them much generic.
> BTW, are there IGT test results for vgem/vkms somewhere? I didn't have
> any luck finding anything.
Unfortunately not (yet). I have this dream that once we have the
kernels on gitlab I'll make igt runs on a qemu run with vkms+vgem only
happen, as the first baseline. But we're some ways off from that still
:-/
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dp: Add function to parse EDID descriptors for adaptive sync limits
On 2019-10-23 8:00 p.m., Manasi Navare wrote:
> Adaptive Sync is a VESA feature so add a DRM core helper to parse
> the EDID's detailed descritors to obtain the adaptive sync monitor range.
> Store this info as part fo drm_display_info so it can be used
> across all drivers.
> This part of the code is stripped out of amdgpu's function
> amdgpu_dm_update_freesync_caps() to make it generic and be used
> across all DRM drivers
>
Please CC Nick on these as well. Added him now.
Would it be possible to add a patch to update amdgpu to call this function?
Harry
> Cc: Ville Syrjälä
> Cc: Harry Wentland
> Cc: Clinton A Taylor
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/drm_edid.c | 49 +
> include/drm/drm_connector.h | 25 +++
> include/drm/drm_edid.h | 2 ++
> 3 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 474ac04d5600..97dd1200773e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4707,6 +4707,52 @@ static void drm_parse_cea_ext(struct drm_connector
> *connector,
> }
> }
>
> +void drm_get_adaptive_sync_limits(struct drm_connector *connector,
> + const struct edid *edid)
> +{
> + struct drm_display_info *info = &connector->display_info;
> + const struct detailed_timing *timing;
> + const struct detailed_non_pixel *data;
> + const struct detailed_data_monitor_range *range;
> + int i;
> +
> + /*
> + * Restrict Adaptive Sync only for dp and edp
> + */
> + if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
> + connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> + return;
> +
> + if (edid->version <= 1 && !(edid->version == 1 && edid->revision > 1))
> + return;
> +
> + for (i = 0; i < 4; i++) {
> + timing = &edid->detailed_timings[i];
> + data= &timing->data.other_data;
> + range = &data->data.range;
> + /*
> + * Check if monitor has continuous frequency mode
> + */
> + if (data->type != EDID_DETAIL_MONITOR_RANGE)
> + continue;
> + /*
> + * Check for flag range limits only. If flag == 1 then
> + * no additional timing information provided.
> + * Default GTF, GTF Secondary curve and CVT are not
> + * supported
> + */
> + if (range->flags != 1)
> + continue;
> +
> + info->adaptive_sync.min_vfreq = range->min_vfreq;
> + info->adaptive_sync.max_vfreq = range->max_vfreq;
> + info->adaptive_sync.pixel_clock_mhz =
> + range->pixel_clock_mhz * 10;
> + break;
> + }
> +}
> +EXPORT_SYMBOL(drm_get_adaptive_sync_limits);
> +
> /* A connector has no EDID information, so we've got no EDID to compute
> quirks from. Reset
> * all of the values which would have been set from EDID
> */
> @@ -4728,6 +4774,7 @@ drm_reset_display_info(struct drm_connector *connector)
> memset(&info->hdmi, 0, sizeof(info->hdmi));
>
> info->non_desktop = 0;
> + memset(&info->adaptive_sync, 0, sizeof(info->adaptive_sync));
> }
>
> u32 drm_add_display_info(struct drm_connector *connector, const struct edid
> *edid)
> @@ -4743,6 +4790,8 @@ u32 drm_add_display_info(struct drm_connector
> *connector, const struct edid *edi
>
> info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
>
> + drm_get_adaptive_sync_limits(connector, edid);
> +
> DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
>
> if (edid->revision < 3)
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 5f8c3389d46f..a27a84270d8d 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -254,6 +254,26 @@ enum drm_panel_orientation {
> DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> };
>
> +/**
> + * struct drm_adaptive_sync_info - Panel's Adaptive Sync capabilities for
> + * &drm_display_info
> + *
> + * This struct is used to store a Panel's Adaptive Sync capabilities
> + * as parsed from EDID's detailed monitor range descriptor block.
> + *
> + * @min_vfreq: This is the min supported refresh rate in Hz from
> + * EDID's detailed monitor range.
> + * @max_vfreq: This is the max supported refresh rate in Hz from
> + * EDID's detailed monitor range
> + * @pixel_clock_mhz: This is the dotclock in MHz from
> + * EDID's detailed monitor range
> + */
> +struct drm_adaptive_sync_info {
> + int min_vfreq;
> + int max_vfreq;
> + int pixel_clock_mhz;
> +};
> +
> /*
> * This is a consolidated colorimetry list supported by HDMI and
> * DP protocol standard. The respective connectors will register
> @@ -465,6 +485,11 @@ struct drm_
[PATCH] drm/tegra: Do not use ->load() and ->unload() callbacks
From: Thierry Reding
The ->load() and ->unload() drivers are midlayers and should be avoided
in modern drivers. Fix this by moving the code into the driver ->probe()
and ->remove() implementations, respectively.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/tegra/drm.c | 386 +---
1 file changed, 186 insertions(+), 200 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 3012f13bab97..bd7a00272965 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -82,202 +82,6 @@ tegra_drm_mode_config_helpers = {
.atomic_commit_tail = tegra_atomic_commit_tail,
};
-static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
-{
- struct host1x_device *device = to_host1x_device(drm->dev);
- struct iommu_domain *domain;
- struct tegra_drm *tegra;
- int err;
-
- tegra = kzalloc(sizeof(*tegra), GFP_KERNEL);
- if (!tegra)
- return -ENOMEM;
-
- /*
-* If the Tegra DRM clients are backed by an IOMMU, push buffers are
-* likely to be allocated beyond the 32-bit boundary if sufficient
-* system memory is available. This is problematic on earlier Tegra
-* generations where host1x supports a maximum of 32 address bits in
-* the GATHER opcode. In this case, unless host1x is behind an IOMMU
-* as well it won't be able to process buffers allocated beyond the
-* 32-bit boundary.
-*
-* The DMA API will use bounce buffers in this case, so that could
-* perhaps still be made to work, even if less efficient, but there
-* is another catch: in order to perform cache maintenance on pages
-* allocated for discontiguous buffers we need to map and unmap the
-* SG table representing these buffers. This is fine for something
-* small like a push buffer, but it exhausts the bounce buffer pool
-* (typically on the order of a few MiB) for framebuffers (many MiB
-* for any modern resolution).
-*
-* Work around this by making sure that Tegra DRM clients only use
-* an IOMMU if the parent host1x also uses an IOMMU.
-*
-* Note that there's still a small gap here that we don't cover: if
-* the DMA API is backed by an IOMMU there's no way to control which
-* device is attached to an IOMMU and which isn't, except via wiring
-* up the device tree appropriately. This is considered an problem
-* of integration, so care must be taken for the DT to be consistent.
-*/
- domain = iommu_get_domain_for_dev(drm->dev->parent);
-
- if (domain && iommu_present(&platform_bus_type)) {
- tegra->domain = iommu_domain_alloc(&platform_bus_type);
- if (!tegra->domain) {
- err = -ENOMEM;
- goto free;
- }
-
- err = iova_cache_get();
- if (err < 0)
- goto domain;
- }
-
- mutex_init(&tegra->clients_lock);
- INIT_LIST_HEAD(&tegra->clients);
-
- drm->dev_private = tegra;
- tegra->drm = drm;
-
- drm_mode_config_init(drm);
-
- drm->mode_config.min_width = 0;
- drm->mode_config.min_height = 0;
-
- drm->mode_config.max_width = 4096;
- drm->mode_config.max_height = 4096;
-
- drm->mode_config.allow_fb_modifiers = true;
-
- drm->mode_config.normalize_zpos = true;
-
- drm->mode_config.funcs = &tegra_drm_mode_config_funcs;
- drm->mode_config.helper_private = &tegra_drm_mode_config_helpers;
-
- err = tegra_drm_fb_prepare(drm);
- if (err < 0)
- goto config;
-
- drm_kms_helper_poll_init(drm);
-
- err = host1x_device_init(device);
- if (err < 0)
- goto fbdev;
-
- if (tegra->group) {
- u64 carveout_start, carveout_end, gem_start, gem_end;
- u64 dma_mask = dma_get_mask(&device->dev);
- dma_addr_t start, end;
- unsigned long order;
-
- start = tegra->domain->geometry.aperture_start & dma_mask;
- end = tegra->domain->geometry.aperture_end & dma_mask;
-
- gem_start = start;
- gem_end = end - CARVEOUT_SZ;
- carveout_start = gem_end + 1;
- carveout_end = end;
-
- order = __ffs(tegra->domain->pgsize_bitmap);
- init_iova_domain(&tegra->carveout.domain, 1UL << order,
-carveout_start >> order);
-
- tegra->carveout.shift = iova_shift(&tegra->carveout.domain);
- tegra->carveout.limit = carveout_end >> tegra->carveout.shift;
-
- drm_mm_init(&tegra->mm, gem_start, gem_end - gem_start + 1);
- mutex_init(&tegra->mm_lock);
-
- DRM_DEBUG_DRIVER("IOMMU apertures:\n");
-
Re: Question regarding "reserved-memory"
On Thu, Oct 24, 2019 at 09:51:04AM -0500, Rob Herring wrote:
> On Thu, Oct 24, 2019 at 9:22 AM Ayan Halder wrote:
Hi Bob,
Thanks for your quick response.
> >
> >
> > Hi Folks,
> >
> > I have a question regarding "reserved-memory". I am using an Arm Juno
> > platform which has a chunk of ram in its fpga. I intend to make this
> > memory as reserved so that it can be shared between various devices
> > for passing framebuffer.
> >
> > My dts looks like the following:-
> >
> > / {
> > // some nodes
> >
> > tlx@6000 {
> > compatible = "simple-bus";
> > ...
> >
> > juno_wrapper {
> >
> > ... /* here we have all the nodes */
> > /* corresponding to the devices in the fpga */
> >
> > memory@d00 {
> >device_type = "memory";
> >reg = <0x00 0x6000 0x00 0x800>;
> > };
> >
> > reserved-memory {
> >#address-cells = <0x01>;
> >#size-cells = <0x01>;
> >ranges;
> >
> >framebuffer@d00 {
> > compatible = "shared-dma-pool";
> > linux,cma-default;
> > reusable;
> > reg = <0x00 0x6000 0x00
> > 0x800>;
> > phandle = <0x44>;
> > };
> > };
> > ...
> > }
> > }
> > ...
> > }
> >
> > Note that the depth of the "reserved-memory" node is 3.
> >
> > Refer __fdt_scan_reserved_mem() :-
> >
> > if (!found && depth == 1 && strcmp(uname, "reserved-memory") == 0) {
> >
> > if (__reserved_mem_check_root(node) != 0) {
> > pr_err("Reserved memory: unsupported node
> > format, ignoring\n");
> > /* break scan */
> > return 1;
> > }
> > found = 1;
> >
> > /* scan next node */
> > return 0;
> > }
> >
> > It expects the "reserved-memory" node to be at depth == 1 and so it
> > does not probe it in our case.
> >
> > Niether from the
> > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > nor from commit - e8d9d1f5485b52ec3c4d7af839e6914438f6c285,
> > I could understand the reason for such restriction.
> >
> > So, I seek the community's advice as to whether I should fix up
> > __fdt_scan_reserved_mem() so as to do away with the restriction or
> > put the "reserved-memory" node outside of 'tlx@6000' (which looks
> > logically incorrect as the memory is on the fpga platform).
>
> For now, I'm going to say it must be at the root level.
Can you mention it in the Documentation/.../reserved-memory.txt,
please?
> I'd guess the
> memory@d00 node is also just ignored (wrong unit-address BTW).
I would request you to ignore the address for the time being. In
juno_wrapper{}, we have a complex remapping of addresses of all the
sub-devices. I deliberately did not put that in the snippet, so as to
prevent any distraction from the core issue.
>
> I think you're also forgetting the later unflattened parsing of
> /reserved-memory.
Are you talking about the remaining part of the
__fdt_scan_reserved_mem() ie
} else if (found && depth < 2) {
/* scanning of /reserved-memory has been finished */
return 1;
}
if (!of_fdt_device_is_available(initial_boot_params, node))
return 0;
err = __reserved_mem_reserve_reg(node, uname);
if (err == -ENOENT && of_get_flat_dt_prop(node, "size", NULL))
fdt_reserved_mem_save_node(node, uname, 0, 0);
/* scan next node */
return 0;
If so, I agree with you that it needs to be changed as well (if we
were to do away with the restriction).
> The other complication IIRC is booting with UEFI
> does it's own reserved memory table which often uses the DT table as
> its source.
I have no knowledge of UEFI booting. So if UEFI expects
"reserved-memory" nodes to be at root level, then we must adhere to
the restriction. :)
Ayan
>
> Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next
Hi Dave & Daniel, Here's the pull for last week and this week. As you know we had some trouble with the OMAP_BO* additions last week, those have since been reverted. Speaking of UAPI, we have a new DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED flag from AMD to get the last signaled timeline value from the kernel. It's used by the AMD implementation of timeline semaphores [1]. The kernel patch was reviewed by Lionel, but the userspace portion was not reviewed in the open (and not even posted before the kernel patch was reviewed). Overall the process was lacking on this submission (as well as the commit message and the kerneldoc), but the addition itself seems fine. Other than that, relatively quiet week overall. [1]- https://github.com/GPUOpen-Drivers/pal/commit/66e78b997748d03d77e1d706c10f1f17e18e5654 drm-misc-next-2019-10-24-2: drm-misc-next for 5.5: UAPI Changes: -syncobj: allow querying the last submitted timeline value (David) -fourcc: explicitly defineDRM_FORMAT_BIG_ENDIAN as unsigned (Adam) -omap: revert the OMAP_BO_* flags that were added -- no userspace (Sean) Cross-subsystem Changes: -MAINTAINERS: add Mihail as komeda co-maintainer (Mihail) Core Changes: -edid: a few cleanups, add AVI infoframe bar info (Ville) -todo: remove i915 device_link item and add difficulty levels (Daniel) -dp_helpers: add a few new helpers to parse dpcd (Thierry) Driver Changes: -gma500: fix a few memory disclosure leaks (Kangjie) -qxl: convert to use the new drm_gem_object_funcs.mmap (Gerd) -various: open code dp_link helpers in preparation for helper removal (Thierry) Cc: Chunming Zhou Cc: Adam Jackson Cc: Sean Paul Cc: Ville Syrjälä Cc: Kangjie Lu Cc: Mihail Atanassov Cc: Daniel Vetter Cc: Thierry Reding Cheers, Sean The following changes since commit 2e79e22e092acd55da0b2db066e4826d7d152c41: Merge v5.4-rc4 into drm-next (2019-10-23 12:10:05 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2019-10-24-2 for you to fetch changes up to 9a42c7c647a9ad0f7ebb147a52eda3dcb7c84292: drm/tegra: Move drm_dp_link helpers to Tegra DRM (2019-10-23 18:22:10 +0200) drm-misc-next for 5.5: UAPI Changes: -syncobj: allow querying the last submitted timeline value (David) -fourcc: explicitly defineDRM_FORMAT_BIG_ENDIAN as unsigned (Adam) -omap: revert the OMAP_BO_* flags that were added -- no userspace (Sean) Cross-subsystem Changes: -MAINTAINERS: add Mihail as komeda co-maintainer (Mihail) Core Changes: -edid: a few cleanups, add AVI infoframe bar info (Ville) -todo: remove i915 device_link item and add difficulty levels (Daniel) -dp_helpers: add a few new helpers to parse dpcd (Thierry) Driver Changes: -gma500: fix a few memory disclosure leaks (Kangjie) -qxl: convert to use the new drm_gem_object_funcs.mmap (Gerd) -various: open code dp_link helpers in preparation for helper removal (Thierry) Cc: Chunming Zhou Cc: Adam Jackson Cc: Sean Paul Cc: Ville Syrjälä Cc: Kangjie Lu Cc: Mihail Atanassov Cc: Daniel Vetter Cc: Thierry Reding Adam Jackson (1): drm/fourcc: Fix undefined left shift in DRM_FORMAT_BIG_ENDIAN macros Andy Shevchenko (1): drm/mipi_dbi: Use simple right shift instead of double negation Ben Dooks (3): drm/scheduler: make unexported items static drm/rockchip: include rockchip_drm_drv.h drm/rockchip: make rockchip_gem_alloc_object static Ben Dooks (Codethink) (1): drm/arm: make undeclared items static Brian Masney (1): drm/bridge: analogix-anx78xx: add support for 7808 addresses Chunming Zhou (1): drm/syncobj: extend syncobj query ability v3 Colin Ian King (1): drm/komeda: remove redundant assignment to pointer disable_done Daniel Kurtz (1): drm/bridge: dw-hdmi: Restore audio when setting a mode Daniel Vetter (4): drm/dp-mst: Drop connection_mutex check drm/doc: Drop misleading comment on drm_mode_config_cleanup drm/todo: Remove i915 device_link task drm/todo: Add levels Dariusz Marcinkiewicz (1): drm: tda998x: use cec_notifier_conn_(un)register Douglas Anderson (1): drm/rockchip: Round up _before_ giving to the clock framework Ezequiel Garcia (2): dt-bindings: display: rockchip: document VOP gamma LUT address drm/rockchip: Add optional support for CRTC gamma LUT Gerd Hoffmann (18): drm: add mmap() to drm_gem_object_funcs drm/shmem: switch shmem helper to &drm_gem_object_funcs.mmap drm/shmem: drop VM_DONTDUMP drm/shmem: drop VM_IO drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS drm/ttm: factor out ttm_bo_mmap_vma_setup drm/ttm: rename ttm_fbdev_mmap drm/ttm: add drm_gem_ttm_mmap() drm/vram: switch vram helper to &drm_gem_object_funcs.mmap() drm/vram: drop verify_access drm/vram: drop DRM_VRAM_MM_FILE_OPERATIONS drm/qx
Re: [PATCH v1 2/3] drm/tegra: Fix 2d and 3d clients detaching from IOMMU domain
On Thu, Oct 24, 2019 at 06:47:23PM +0300, Dmitry Osipenko wrote:
> 24.10.2019 16:50, Thierry Reding пишет:
> > On Thu, Oct 24, 2019 at 04:28:41PM +0300, Dmitry Osipenko wrote:
> >> 24.10.2019 14:58, Thierry Reding пишет:
> >>> On Sun, Jun 23, 2019 at 08:37:42PM +0300, Dmitry Osipenko wrote:
> This should should fire up on the DRM's driver module re-loader because
> there won't be enough available domains on older Tegra SoCs.
>
> Cc: stable
> Fixes: 0c407de5ed1a ("drm/tegra: Refactor IOMMU attach/detach")
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/gpu/drm/tegra/dc.c | 4 ++--
> drivers/gpu/drm/tegra/drm.c | 9 ++---
> drivers/gpu/drm/tegra/drm.h | 3 ++-
> drivers/gpu/drm/tegra/gr2d.c | 4 ++--
> drivers/gpu/drm/tegra/gr3d.c | 4 ++--
> 5 files changed, 14 insertions(+), 10 deletions(-)
> >>>
> >>> I think I understand what this is trying to do, but the commit message
> >>> does not help at all. So what's really going on here is that we need to
> >>> detach the device from the group regardless of whether we're sharing the
> >>> group or not, just like we attach groups to the shared domain whether
> >>> they share the same group or not.
> >>
> >> Yes, the commit's message could be improved.
> >>
> >>> But in that case, I wonder if it's even worth splitting groups the way
> >>> we are right now. Wouldn't it be better to just put all the devices into
> >>> the same group and be done with it?
> >>>
> >>> The current code gives me headaches every time I read it, so if we can
> >>> just make it so that all the devices under the DRM device share the same
> >>> group, this would become a lot easier to deal with. I'm not really
> >>> convinced that it makes much sense to keep them on separate domains,
> >>> especially given the constraints on the number of domains available on
> >>> earlier Tegra devices.
> >>>
> >>> Note that sharing a group will also make it much easier for these to use
> >>> the DMA API if it is backed by an IOMMU.
> >>
> >> Probably I'm blanking on everything about IOMMU now.. could you please
> >> remind me what "IOMMU group" is?
> >>
> >> Isn't it that each IOMMU group relates to the HW ID (SWGROUP)? But then
> >> each display controller has its own SWGROUP.. and thus that sharing just
> >> doesn't make any sense, hm.
> >
> > IOMMU groups are not directly related to SWGROUPs. But by default the
> > IOMMU framework will share a domain between members of the same IOMMU
> > group.
>
> Ah, I re-figured out that again. The memory controller drivers are
> defining a single "IOMMU group" for both of the display controllers.
>
> > Seems like that's really what we want here, so that when we do
> > use the DMA API, all the devices part of the DRM device get attached to
> > the same IOMMU domain, yet if we don't want to use the DMA API we only
> > need to detach the one group from the backing.
>
> Yes, it should be okay to put all DRM devices into the same group, like
> it is done now for the displays. It also should resolve problem with the
> domains shortage on T30 since now there are maximum 3 domains in use:
> host1x, drm and vde.
>
> I actually just checked that the original problem still exists
> and this change solves it as well:
>
> ---
> diff --git a/drivers/memory/tegra/tegra30.c b/drivers/memory/tegra/tegra30.c
> index 5a0f6e0a1643..e71096498436 100644
> --- a/drivers/memory/tegra/tegra30.c
> +++ b/drivers/memory/tegra/tegra30.c
> @@ -1021,6 +1021,9 @@ static const struct tegra_smmu_swgroup
> tegra30_swgroups[] = {
> static const unsigned int tegra30_group_display[] = {
> TEGRA_SWGROUP_DC,
> TEGRA_SWGROUP_DCB,
> + TEGRA_SWGROUP_G2,
> + TEGRA_SWGROUP_NV,
> + TEGRA_SWGROUP_NV2,
> };
>
> static const struct tegra_smmu_group_soc tegra30_groups[] = {
> ---
>
> Please let me know whether you're going to make a patch or if I should
> do it.
I've been testing with a similar change and couldn't find any
regressions. I've also made the same modifications for Tegra114 and
Tegra124.
Are you saying that none of these patches are needed anymore? Or do we
still need a patch to fix detaching? I'm thinking that maybe we can
drastrically simplify the detachment now by dropping the shared
parameter altogether.
Let me draft a patch and send out the whole set for testing.
Thierry
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH trivial] drm: Spelling s/connet/connect/
On Thu, Oct 24, 2019 at 05:17:37PM +0200, Geert Uytterhoeven wrote:
> Fix misspellings of "connector" and "connection"
>
> Signed-off-by: Geert Uytterhoeven
Thanks, applied to drm-misc-next.
-Daniel
> ---
> drivers/gpu/drm/gma500/mdfld_dsi_output.c | 2 +-
> include/uapi/drm/exynos_drm.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> index 03023fa0fb6f9d3c..f350ac1ead18213e 100644
> --- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> +++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> @@ -498,7 +498,7 @@ void mdfld_dsi_output_init(struct drm_device *dev,
> return;
> }
>
> - /*create a new connetor*/
> + /*create a new connector*/
> dsi_connector = kzalloc(sizeof(struct mdfld_dsi_connector), GFP_KERNEL);
> if (!dsi_connector) {
> DRM_ERROR("No memory");
> diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h
> index 3e59b8382dd8cead..45c6582b3df31dbf 100644
> --- a/include/uapi/drm/exynos_drm.h
> +++ b/include/uapi/drm/exynos_drm.h
> @@ -68,7 +68,7 @@ struct drm_exynos_gem_info {
> /**
> * A structure for user connection request of virtual display.
> *
> - * @connection: indicate whether doing connetion or not by user.
> + * @connection: indicate whether doing connection or not by user.
> * @extensions: if this value is 1 then the vidi driver would need additional
> * 128bytes edid data.
> * @edid: the edid data pointer from user side.
> --
> 2.17.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205277] [amd powerplay] vega10: soc voltage for power state 7 is not changed by overdrive.
https://bugzilla.kernel.org/show_bug.cgi?id=205277 --- Comment #6 from Pelle van Gils (pe...@vangils.xyz) --- Created attachment 285633 --> https://bugzilla.kernel.org/attachment.cgi?id=285633&action=edit proposed patch v2 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/tegra: Do not use ->load() and ->unload() callbacks
On Thu, Oct 24, 2019 at 05:10:30PM +0200, Thierry Reding wrote:
> From: Thierry Reding
>
> The ->load() and ->unload() drivers are midlayers and should be avoided
> in modern drivers. Fix this by moving the code into the driver ->probe()
> and ->remove() implementations, respectively.
>
> Signed-off-by: Thierry Reding
> ---
> drivers/gpu/drm/tegra/drm.c | 386 +---
> 1 file changed, 186 insertions(+), 200 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 3012f13bab97..bd7a00272965 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -82,202 +82,6 @@ tegra_drm_mode_config_helpers = {
> .atomic_commit_tail = tegra_atomic_commit_tail,
> };
>
> -static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
> -{
> - struct host1x_device *device = to_host1x_device(drm->dev);
> - struct iommu_domain *domain;
> - struct tegra_drm *tegra;
> - int err;
> -
> - tegra = kzalloc(sizeof(*tegra), GFP_KERNEL);
> - if (!tegra)
> - return -ENOMEM;
> -
> - /*
> - * If the Tegra DRM clients are backed by an IOMMU, push buffers are
> - * likely to be allocated beyond the 32-bit boundary if sufficient
> - * system memory is available. This is problematic on earlier Tegra
> - * generations where host1x supports a maximum of 32 address bits in
> - * the GATHER opcode. In this case, unless host1x is behind an IOMMU
> - * as well it won't be able to process buffers allocated beyond the
> - * 32-bit boundary.
> - *
> - * The DMA API will use bounce buffers in this case, so that could
> - * perhaps still be made to work, even if less efficient, but there
> - * is another catch: in order to perform cache maintenance on pages
> - * allocated for discontiguous buffers we need to map and unmap the
> - * SG table representing these buffers. This is fine for something
> - * small like a push buffer, but it exhausts the bounce buffer pool
> - * (typically on the order of a few MiB) for framebuffers (many MiB
> - * for any modern resolution).
> - *
> - * Work around this by making sure that Tegra DRM clients only use
> - * an IOMMU if the parent host1x also uses an IOMMU.
> - *
> - * Note that there's still a small gap here that we don't cover: if
> - * the DMA API is backed by an IOMMU there's no way to control which
> - * device is attached to an IOMMU and which isn't, except via wiring
> - * up the device tree appropriately. This is considered an problem
> - * of integration, so care must be taken for the DT to be consistent.
> - */
> - domain = iommu_get_domain_for_dev(drm->dev->parent);
> -
> - if (domain && iommu_present(&platform_bus_type)) {
> - tegra->domain = iommu_domain_alloc(&platform_bus_type);
> - if (!tegra->domain) {
> - err = -ENOMEM;
> - goto free;
> - }
> -
> - err = iova_cache_get();
> - if (err < 0)
> - goto domain;
> - }
> -
> - mutex_init(&tegra->clients_lock);
> - INIT_LIST_HEAD(&tegra->clients);
> -
> - drm->dev_private = tegra;
> - tegra->drm = drm;
> -
> - drm_mode_config_init(drm);
> -
> - drm->mode_config.min_width = 0;
> - drm->mode_config.min_height = 0;
> -
> - drm->mode_config.max_width = 4096;
> - drm->mode_config.max_height = 4096;
> -
> - drm->mode_config.allow_fb_modifiers = true;
> -
> - drm->mode_config.normalize_zpos = true;
> -
> - drm->mode_config.funcs = &tegra_drm_mode_config_funcs;
> - drm->mode_config.helper_private = &tegra_drm_mode_config_helpers;
> -
> - err = tegra_drm_fb_prepare(drm);
> - if (err < 0)
> - goto config;
> -
> - drm_kms_helper_poll_init(drm);
> -
> - err = host1x_device_init(device);
> - if (err < 0)
> - goto fbdev;
> -
> - if (tegra->group) {
> - u64 carveout_start, carveout_end, gem_start, gem_end;
> - u64 dma_mask = dma_get_mask(&device->dev);
> - dma_addr_t start, end;
> - unsigned long order;
> -
> - start = tegra->domain->geometry.aperture_start & dma_mask;
> - end = tegra->domain->geometry.aperture_end & dma_mask;
> -
> - gem_start = start;
> - gem_end = end - CARVEOUT_SZ;
> - carveout_start = gem_end + 1;
> - carveout_end = end;
> -
> - order = __ffs(tegra->domain->pgsize_bitmap);
> - init_iova_domain(&tegra->carveout.domain, 1UL << order,
> - carveout_start >> order);
> -
> - tegra->carveout.shift = iova_shift(&tegra->carveout.domain);
> - tegra->carveout.limit = carveout_end >> tegra->carveout.shift;
> -
> - drm_mm_init(&tegra->mm, gem_s
[Bug 205277] [amd powerplay] vega10: soc voltage for power state 7 is not changed by overdrive.
https://bugzilla.kernel.org/show_bug.cgi?id=205277 --- Comment #7 from Pelle van Gils (pe...@vangils.xyz) --- (In reply to haro41 from comment #3) > Did you debug this issue? I think the problem could be outside this code. > > I would outcomment the if-statement following for-loop in your proposed > patch, because otherwise 'i' points outside the array boundarys here. Thank you for your reply. I have uploaded a new patch with your suggestion. It looks to me now that this is not so much a bug but intended beviour. I would still like to see this changed though. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205277] [amd powerplay] vega10: soc voltage for power state 7 is not changed by overdrive.
https://bugzilla.kernel.org/show_bug.cgi?id=205277 Pelle van Gils (pe...@vangils.xyz) changed: What|Removed |Added Kernel Version|5.4.0-rc3 |5.4.0-rc4 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #153 from Marko Popovic --- (In reply to L.S.S. from comment #152) > UPDATE: I just got another freeze on 5.3.6 kernel. The same > GCVM_L2_PROTECTION_FAULT error followed by a ring sdma0 timeout. > > So it seems AMD_DEBUG="nodma nongg" doesn't really work for me. Can you at least provide the dmesg log so we can determine what type of hang you're having and directing you to the right bugtracker, since there are multiple types. This also varies greatly from one desktop environment to other, wayland or not etc. This topic is mostly concerning the SDMA type hangs that happen at random, and AMD_DEBUG=nodma seems to take care of it for almost anyone, I don't think using nongg is neccessary since until now it's only been proven to take care of 1 specific hang happening in Citra emulator, which is also ring-gfx type so it's a driver bug, probably not kernel driver related. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RESEND PATCH v4] drm: Don't free jobs in wait_event_interruptible()
drm_sched_cleanup_jobs() attempts to free finished jobs, however because
it is called as the condition of wait_event_interruptible() it must not
sleep. Unfortuantly some free callbacks (notibly for Panfrost) do sleep.
Instead let's rename drm_sched_cleanup_jobs() to
drm_sched_get_cleanup_job() and simply return a job for processing if
there is one. The caller can then call the free_job() callback outside
the wait_event_interruptible() where sleeping is possible before
re-checking and returning to sleep if necessary.
Signed-off-by: Steven Price
---
Previous posting:
https://lore.kernel.org/lkml/20190926141630.14258-1-steven.pr...@arm.com/
drivers/gpu/drm/scheduler/sched_main.c | 45 +++---
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 9a0ee74d82dc..148468447ba9 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -622,43 +622,41 @@ static void drm_sched_process_job(struct dma_fence *f,
struct dma_fence_cb *cb)
}
/**
- * drm_sched_cleanup_jobs - destroy finished jobs
+ * drm_sched_get_cleanup_job - fetch the next finished job to be destroyed
*
* @sched: scheduler instance
*
- * Remove all finished jobs from the mirror list and destroy them.
+ * Returns the next finished job from the mirror list (if there is one)
+ * ready for it to be destroyed.
*/
-static void drm_sched_cleanup_jobs(struct drm_gpu_scheduler *sched)
+static struct drm_sched_job *
+drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched)
{
+ struct drm_sched_job *job = NULL;
unsigned long flags;
/* Don't destroy jobs while the timeout worker is running */
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
!cancel_delayed_work(&sched->work_tdr))
- return;
-
+ return NULL;
- while (!list_empty(&sched->ring_mirror_list)) {
- struct drm_sched_job *job;
+ spin_lock_irqsave(&sched->job_list_lock, flags);
- job = list_first_entry(&sched->ring_mirror_list,
+ job = list_first_entry_or_null(&sched->ring_mirror_list,
struct drm_sched_job, node);
- if (!dma_fence_is_signaled(&job->s_fence->finished))
- break;
- spin_lock_irqsave(&sched->job_list_lock, flags);
+ if (job && dma_fence_is_signaled(&job->s_fence->finished)) {
/* remove job from ring_mirror_list */
list_del_init(&job->node);
- spin_unlock_irqrestore(&sched->job_list_lock, flags);
-
- sched->ops->free_job(job);
+ } else {
+ job = NULL;
+ /* queue timeout for next job */
+ drm_sched_start_timeout(sched);
}
- /* queue timeout for next job */
- spin_lock_irqsave(&sched->job_list_lock, flags);
- drm_sched_start_timeout(sched);
spin_unlock_irqrestore(&sched->job_list_lock, flags);
+ return job;
}
/**
@@ -698,12 +696,21 @@ static int drm_sched_main(void *param)
struct drm_sched_fence *s_fence;
struct drm_sched_job *sched_job;
struct dma_fence *fence;
+ struct drm_sched_job *cleanup_job = NULL;
wait_event_interruptible(sched->wake_up_worker,
-(drm_sched_cleanup_jobs(sched),
+(cleanup_job =
drm_sched_get_cleanup_job(sched)) ||
(!drm_sched_blocked(sched) &&
(entity =
drm_sched_select_entity(sched))) ||
-kthread_should_stop()));
+kthread_should_stop());
+
+ while (cleanup_job) {
+ sched->ops->free_job(cleanup_job);
+ /* queue timeout for next job */
+ drm_sched_start_timeout(sched);
+
+ cleanup_job = drm_sched_get_cleanup_job(sched);
+ }
if (!entity)
continue;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
