Re: [PATCH] drm/panfrost: Really power off GPU cores in panfrost_gpu_power_off()
On 22/11/2023 10:29, Krzysztof Kozlowski wrote:
> On 22/11/2023 10:06, AngeloGioacchino Del Regno wrote:
>
>>
>
> Hey Krzysztof,
>
> This is interesting. It might be about the cores that are missing from
> the partial
> core_mask raising interrupts, but an external abort on non-linefetch is
> strange to
> see here.
I've seen such external aborts in the past, and the fault type has
often been misleading. It's unlikely to have anything to do with a
>>>
>>> Yeah, often accessing device with power or clocks gated.
>>>
>>
>> Except my commit does *not* gate SoC power, nor SoC clocks
>
> It could be that something (like clocks or power supplies) was missing
> on this board/SoC, which was not critical till your patch came.
>
>>
>> What the "Really power off ..." commit does is to ask the GPU to internally
>> power
>> off the shaders, tilers and L2, that's why I say that it is strange to see
>> that
>> kind of abort.
>>
>> The GPU_INT_CLEAR GPU_INT_STAT, GPU_FAULT_STATUS and
>> GPU_FAULT_ADDRESS_{HI/LO}
>> registers should still be accessible even with shaders, tilers and cache OFF.
>>
>> Anyway, yes, synchronizing IRQs before calling the poweroff sequence would
>> also
>> work, but that'd add up quite a bit of latency on the runtime_suspend()
>> call, so
>> in this case I'd be more for avoiding to execute any register r/w in the
>> handler
>> by either checking if the GPU is supposed to be OFF, or clearing interrupts,
>> which
>> may not work if those are generated after the execution of the poweroff
>> function.
>> Or we could simply disable the irq after power_off, but that'd be hacky (as
>> well).
>>
>>
>> Let's see if asking to poweroff *everything* works:
>
> Worked.
Is anything happening with this patchset? linux-next is broken - fails
to boot - for three weeks now on my platforms.
Best regards,
Krzysztof
[PATCH] drm/imagination: move update_logtype() into ifdef section
From: Arnd Bergmann
This function is only used when debugfs is enabled, and otherwise
causes a build warning:
drivers/gpu/drm/imagination/pvr_fw_trace.c:135:1: error: 'update_logtype'
defined but not used [-Werror=unused-function]
Move the #ifdef check to include this function as well.
Fixes: cb56cd610866 ("drm/imagination: Add firmware trace to debugfs")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/imagination/pvr_fw_trace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.c
b/drivers/gpu/drm/imagination/pvr_fw_trace.c
index 87a42fb6ace6..8261fa4f7f83 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_trace.c
+++ b/drivers/gpu/drm/imagination/pvr_fw_trace.c
@@ -121,6 +121,8 @@ void pvr_fw_trace_fini(struct pvr_device *pvr_dev)
pvr_fw_object_unmap_and_destroy(fw_trace->tracebuf_ctrl_obj);
}
+#if defined(CONFIG_DEBUG_FS)
+
/**
* update_logtype() - Send KCCB command to trigger FW to update logtype
* @pvr_dev: Target PowerVR device
@@ -165,8 +167,6 @@ update_logtype(struct pvr_device *pvr_dev, u32 group_mask)
return err;
}
-#if defined(CONFIG_DEBUG_FS)
-
static int fw_trace_group_mask_show(struct seq_file *m, void *data)
{
struct pvr_device *pvr_dev = m->private;
--
2.39.2
[PATCH] drm/bridge: tc358768: select CONFIG_VIDEOMODE_HELPERS
From: Arnd Bergmann
A dependency on this feature was recently introduced:
x86_64-linux-ld: vmlinux.o: in function `tc358768_bridge_pre_enable':
tc358768.c:(.text+0xbe3dae): undefined reference to
`drm_display_mode_to_videomode'
Make sure this is always enabled.
Fixes: e5fb21678136 ("drm/bridge: tc358768: Use struct videomode")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/bridge/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index ba82a1142adf..3e6a4e2044c0 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -313,6 +313,7 @@ config DRM_TOSHIBA_TC358768
select REGMAP_I2C
select DRM_PANEL
select DRM_MIPI_DSI
+ select VIDEOMODE_HELPERS
help
Toshiba TC358768AXBG/TC358778XBG DSI bridge chip driver.
--
2.39.2
linux-next: build warnings after merge of the drm-misc tree
Hi all,
After merging the drm-misc tree, today's linux-next build (htmldocs)
produced these warnings:
include/drm/drm_plane.h:60: warning: expecting prototype for struct
solid_fill_property. Prototype was for struct drm_solid_fill instead
include/drm/drm_plane.h:833: warning: Function parameter or member
'pixel_source_property' not described in 'drm_plane'
Introduced by commits
e50e5fed41c7 ("drm: Introduce pixel_source DRM plane property")
85863a4e16e7 ("drm: Introduce solid fill DRM plane property")
--
Cheers,
Stephen Rothwell
pgpG9WQIQDt_H.pgp
Description: OpenPGP digital signature
Re: [PATCH v3 2/2] drm/msm/dpu: Add mutex lock in control vblank irq
On Fri, Dec 01, 2023 at 11:43:36AM -0800, Abhinav Kumar wrote: > > > On 12/1/2023 8:22 AM, Bjorn Andersson wrote: > > On Fri, Dec 01, 2023 at 10:34:50AM +0200, Dmitry Baryshkov wrote: > > > On Fri, 1 Dec 2023 at 05:47, Bjorn Andersson > > > wrote: > > > > On Thu, Nov 30, 2023 at 05:40:55PM -0800, Paloma Arellano wrote: > > [..] > > > > > @@ -2386,6 +2390,7 @@ struct drm_encoder *dpu_encoder_init(struct > > > > > drm_device *dev, > > > > >dpu_enc->enabled = false; > > > > >mutex_init(_enc->enc_lock); > > > > >mutex_init(_enc->rc_lock); > > > > > + mutex_init(_enc->vblank_ctl_lock); > > > > > > > > Is this somehow propagated to multiple different dpu_encoder_phys > > > > instances, or why do you need to initialize it here and pass the pointer > > > > through 2 different intermediate structures before assigning it to > > > > phys_enc->vblank_ctl_lock below? > > > > > > Yes, there can be two phys_enc instances for a single encoder, so this > > > part is fine. > > > > > > > Thanks for the clarification, Dmitry. Sounds like it make sense then. > > > > But, if I read the code correctly the two instances will have separate > > vblank_refcount copies, and the dpu_core_irq_*() interface does mutual > > exclusion within. So why do we need shared mutual exclusion between the > > two? (This is where a proper description of the problem in the commit > > message would have been very helpful) > > > > Are you suggesting we just have one vblank_ctl_lock per encoder and not have > one vblank_ctl_lock per phys encoder? I cannot think of a display specific > reason for that other than just the SW layout. > > The reason its like this today is that control_vblank_irq is an encoder phys > op because it does different things based on the type of encoder. > > Because its an encoder phys op, it has the vblank_ctl_lock at the phys > structure and not the encoder one. > > Its just more about how the phys op is defined that each phys op operates on > its phys's structure. > > Generally, if we have one encoder with two physical encoders we anyways bail > out early for the other encoder so this is mostly a no-op for the slave phys > encoder. > > Please take a look at below return point. > > 715 /* Slave encoders don't report vblank */ > 716 if (!sde_encoder_phys_vid_is_master(phys_enc)) > 717 goto end; > 718 > > So technically its still providing protection for the same phys encoder but > the catch is this control_vblank_irq can get called from different threads > hence we need exclusion. > The way I understand the code is that the atomic is used to refcount when to enable/disable the interrupt, and the new lock protects this refcount during concurrent updates. I have no concerns with this part. What I'm seeing is that the refcount it per phys_enc, and as such there would be no reason to have a common mutex to protect the two independent refcounts. But I'm probably misunderstanding something here... Regards, Bjorn
Re: [PATCH v2 09/10] drm/bridge: tc358775: Add support for tc358765
On Sat, 2 Dec 2023 at 10:04, Tony Lindgren wrote:
>
> The tc358775 bridge is pin compatible with earlier tc358765 according to
> the tc358774xbg_datasheet_en_20190118.pdf documentation. Compared to the
> tc358765, the tc358775 supports a STBY GPIO and higher data rates.
>
> The tc358765 has a register bit for video event mode vs video pulse mode.
> We must set it to video event mode for the LCD output to work, and on the
> tc358775, this bit no longer exists.
>
> Looks like the registers seem to match otherwise based on a quick glance
> comparing the defines to the earlier Android kernel tc358765 driver.
>
> Signed-off-by: Tony Lindgren
> ---
> drivers/gpu/drm/bridge/tc358775.c | 26 ++
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358775.c
> b/drivers/gpu/drm/bridge/tc358775.c
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -107,6 +108,7 @@
> #define RDPKTLN 0x0404 /* Command Read Packet Length */
>
> #define VPCTRL 0x0450 /* Video Path Control */
> +#define EVTMODEBIT(5) /* Video event mode enable, tc35876x
> only */
> #define HTIM1 0x0454 /* Horizontal Timing Control 1 */
> #define HTIM2 0x0458 /* Horizontal Timing Control 2 */
> #define VTIM1 0x045C /* Vertical Timing Control 1 */
> @@ -254,6 +256,11 @@ enum tc358775_ports {
> TC358775_LVDS_OUT1,
> };
>
> +enum tc3587x5_type {
> + TC358765,
I'd suggest adding = 1 or =0x65 so that the specified type differs
from 'no data' = 0 / NULL.
> + TC358775,
> +};
> +
> struct tc_data {
> struct i2c_client *i2c;
> struct device *dev;
> @@ -271,6 +278,8 @@ struct tc_data {
> struct gpio_desc*stby_gpio;
> u8 lvds_link; /* single-link or dual-link */
> u8 bpc;
> +
> + enum tc3587x5_type type;
> };
>
> static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
> @@ -424,10 +433,16 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
> d2l_write(tc->i2c, PPI_STARTPPI, PPI_START_FUNCTION);
> d2l_write(tc->i2c, DSI_STARTDSI, DSI_RX_START);
>
> + /* Video event mode vs pulse mode bit, does not exist for tc358775 */
> + if (tc->type == TC358765)
> + val = EVTMODE;
> + else
> + val = 0;
> +
> if (tc->bpc == 8)
> - val = TC358775_VPCTRL_OPXLFMT(1);
> + val |= TC358775_VPCTRL_OPXLFMT(1);
> else /* bpc = 6; */
> - val = TC358775_VPCTRL_MSF(1);
> + val |= TC358775_VPCTRL_MSF(1);
>
> dsiclk = mode->crtc_clock * 3 * tc->bpc / tc->num_dsi_lanes / 1000;
> clkdiv = dsiclk / (tc->lvds_link == DUAL_LINK ? DIVIDE_BY_6 :
> DIVIDE_BY_3);
> @@ -643,6 +658,7 @@ static int tc_probe(struct i2c_client *client)
>
> tc->dev = dev;
> tc->i2c = client;
> + tc->type = (enum tc3587x5_type)of_device_get_match_data(dev);
Would it make sense to use i2c_get_match_data() instead?
>
> tc->panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node,
> TC358775_LVDS_OUT0, 0);
> @@ -704,13 +720,15 @@ static void tc_remove(struct i2c_client *client)
> }
>
> static const struct i2c_device_id tc358775_i2c_ids[] = {
> - { "tc358775", 0 },
> + { "tc358765", TC358765, },
> + { "tc358775", TC358775, },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, tc358775_i2c_ids);
>
> static const struct of_device_id tc358775_of_ids[] = {
> - { .compatible = "toshiba,tc358775", },
> + { .compatible = "toshiba,tc358765", .data = (void *)TC358765, },
> + { .compatible = "toshiba,tc358775", .data = (void *)TC358775, },
> { }
> };
> MODULE_DEVICE_TABLE(of, tc358775_of_ids);
> --
> 2.43.0
--
With best wishes
Dmitry
Re: [PATCH v2 10/10] drm/bridge: tc358775: Configure hs_rate and lp_rate
On Sat, 2 Dec 2023 at 10:06, Tony Lindgren wrote: > > The hs_rate and lp_rate may be used by the dsi host for timing > calculations. The tc358775 has a maximum bit rate of 1 Gbps/lane, > tc358765 has maximurate of 800 Mbps per lane. > > Signed-off-by: Tony Lindgren > --- > drivers/gpu/drm/bridge/tc358775.c | 5 + > 1 file changed, 5 insertions(+) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v2 08/10] drm/bridge: tc358775: Enable pre_enable_prev_first flag
On Sat, 2 Dec 2023 at 10:03, Tony Lindgren wrote: > > Set pre_enable_prev_first to ensure the previous bridge is enabled > first. > > Signed-off-by: Tony Lindgren > --- > drivers/gpu/drm/bridge/tc358775.c | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v2 06/10] drm/bridge: tc358775: Get bridge data lanes instead of the DSI host lanes
On Sat, 2 Dec 2023 at 10:01, Tony Lindgren wrote: > > The current code assumes the data-lanes property is configured on the > DSI host side instead of the bridge side, and assumes DSI host endpoint 1. > > Let's standardize on what the other bridge drivers are doing and parse the > data-lanes property for the bridge. Only if data-lanes property is not found, > let's be nice and also check the DSI host for old dtb in use and warn. It might be worth adding that lanes configuration for the host and for the bridge might be different (e.g. the lanes might be swapped on the host side). Reviewed-by: Dmitry Baryshkov > Signed-off-by: Tony Lindgren > --- > drivers/gpu/drm/bridge/tc358775.c | 25 +++-- > 1 file changed, 11 insertions(+), 14 deletions(-) -- With best wishes Dmitry
Re: [PATCH v2 05/10] drm/bridge: tc358775: make standby GPIO optional
On Sat, 2 Dec 2023 at 10:01, Tony Lindgren wrote: > > From: Michael Walle > > The stby pin is optional. It is only needed for power-up and down > sequencing. It is not needed, if the power rails cannot by dynamically > enabled. > > Because the GPIO is not optional, remove the error message. > > Signed-off-by: Michael Walle > Signed-off-by: Tony Lindgren > --- > drivers/gpu/drm/bridge/tc358775.c | 9 +++-- > 1 file changed, 3 insertions(+), 6 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [RFC PATCH 0/6] Supporting GMEM (generalized memory management) for external memory devices
Christian König writes: > Am 01.12.23 um 06:48 schrieb Zeng, Oak: >> [SNIP] >> Besides memory eviction/oversubscription, there are a few other pain points >> when I use hmm: >> >> 1) hmm doesn't support file-back memory, so it is hard to share > memory b/t process in a gpu environment. You mentioned you have a > plan... How hard is it to support file-backed in your approach? > > As hard as it is to support it through HMM. That's what I meant that > this approach doesn't integrate well, as far as I know the problem > isn't inside HMM or any other solution but rather in the file system > layer. In what way does HMM not support file-backed memory? I was under the impression that at least hmm_range_fault() does. - Alistair > Regards, > Christian. > >> 2)virtual address range based memory attribute/hint: with hmadvise, > where do you save the memory attribute of a virtual address range? Do > you need to extend vm_area_struct to save it? With hmm, we have to > maintain such information at driver. This ends up with pretty > complicated logic to split/merge those address range. I know core mm > has similar logic to split/merge vma... >> >> Oak >> >> >>> -Weixi >>> >>> -Original Message- >>> From: Christian König >>> Sent: Thursday, November 30, 2023 4:28 PM >>> To: Zeng, Oak; Christian König >>> ; zhuweixi; linux- >>> m...@kvack.org;linux-ker...@vger.kernel.org;a...@linux-foundation.org; >>> Danilo Krummrich; Dave Airlie; Daniel >>> Vetter >>> Cc:intel-gvt-...@lists.freedesktop.org;rcampb...@nvidia.com; >>> mhairgr...@nvidia.com;j...@nvidia.com;weixi@openeuler.sh; >>> jhubb...@nvidia.com;intel-...@lists.freedesktop.org;apop...@nvidia.com; >>> xinhui@amd.com;amd-...@lists.freedesktop.org; >>> tvrtko.ursu...@linux.intel.com;ogab...@kernel.org;jgli...@redhat.com; dri- >>> de...@lists.freedesktop.org;z...@nvidia.com; Vivi, Rodrigo >>> ;alexander.deuc...@amd.com;leo...@nvidia.com; >>> felix.kuehl...@amd.com; Wang, Zhi A; >>> mgor...@suse.de >>> Subject: Re: [RFC PATCH 0/6] Supporting GMEM (generalized memory >>> management) for external memory devices >>> >>> Hi Oak, >>> >>> yeah, #4 is indeed a really good point and I think Felix will agree to that >>> as well. >>> >>> HMM is basically still missing a way to advise device attributes for the CPU >>> address space. Both migration strategy as well as device specific >>> information (like >>> cache preferences) fall into this category. >>> >>> Since there is a device specific component in those attributes as well I >>> think >>> device specific IOCTLs still make sense to update them, but HMM should offer >>> the functionality to manage and store those information. >>> >>> Split and merge of VMAs only become a problem if you attach those >>> information >>> to VMAs, if you keep them completely separate than that doesn't become an >>> issue either. The down side of this approach is that you don't get >>> automatically >>> extending attribute ranges for growing VMAs for example. >>> >>> Regards, >>> Christian. >>> >>> Am 29.11.23 um 23:23 schrieb Zeng, Oak: Hi Weixi, Even though Christian has listed reasons rejecting this proposal (yes they are >>> very reasonable to me), I would open my mind and further explore the >>> possibility >>> here. Since the current GPU driver uses a hmm based implementation (AMD and >>> NV has done this; At Intel we are catching up), I want to explore how much >>> we >>> can benefit from the proposed approach and how your approach can solve some >>> pain points of our development. So basically what I am questioning here is: >>> what >>> is the advantage of your approach against hmm. To implement a UVM (unified virtual address space b/t cpu and gpu device), >>> with hmm, driver essentially need to implement below functions: 1. device page table update. Your approach requires the same because this is device specific codes 2. Some migration functions to migrate memory b/t system memory and GPU >>> local memory. My understanding is, even though you generalized this a bit, >>> such >>> as modified cpu page fault path, provided "general" gm_dev_fault handler... >>> but >>> device driver still need to provide migration functions because migration >>> functions have to be device specific (i.e., using device dma/copy engine for >>> performance purpose). Right? 3. GPU physical memory management, this part is now in drm/buddy, shared >>> by all drivers. I think with your approach, driver still need to provide >>> callback >>> functions to allocate/free physical pages. Right? Or do you let linux core >>> mm >>> buddy manage device memory directly? 4. madvise/hints/virtual address range management. This has been pain point >>> for us. Right now device driver has to maintain certain virtual address >>> range data >>> structure to maintain hints and other virtual address range based memory >>> attributes. Driver need to sync with linux vma. Driver need to explicitly
Re: [PATCH 2/2] drm/panel: simple: Add BOE BP082WX1-100 8.2" panel
On Sat, 2 Dec 2023 at 10:13, Tony Lindgren wrote:
>
> The BOE BP082WX1-100 is a 8.2" panel similar to the 10.1" panel
> BP101WX1-100. Both panels use the same timings.
>
> Signed-off-by: Tony Lindgren
Reviewed-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/panel/panel-simple.c | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1336,6 +1336,23 @@ static const struct drm_display_mode
> boe_bp101wx1_100_mode = {
> .vtotal = 800 + 6 + 8 + 2,
> };
>
> +static const struct panel_desc boe_bp082wx1_100 = {
> + .modes = _bp101wx1_100_mode,
> + .num_modes = 1,
> + .bpc = 8,
> + .size = {
> + .width = 180,
> + .height = 114,
Nit: Panelook gives following dimensions: 176.64(W)×110.4(H) mm
> + },
> + .delay = {
> + .enable = 50,
> + .disable = 50,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA,
> + .bus_flags = DRM_BUS_FLAG_DE_HIGH,
> + .connector_type = DRM_MODE_CONNECTOR_LVDS,
> +};
> +
> static const struct panel_desc boe_bp101wx1_100 = {
> .modes = _bp101wx1_100_mode,
> .num_modes = 1,
> @@ -4281,6 +4298,9 @@ static const struct of_device_id platform_of_match[] = {
> }, {
> .compatible = "bananapi,s070wv20-ct16",
> .data = _s070wv20_ct16,
> + }, {
> + .compatible = "boe,bp082wx1-100",
> + .data = _bp082wx1_100,
> }, {
> .compatible = "boe,bp101wx1-100",
> .data = _bp101wx1_100,
> --
> 2.43.0
--
With best wishes
Dmitry
Re: [PATCH v3] drm/atomic-helpers: Invoke end_fb_access while owning plane state
Thomas Zimmermann writes:
> Invoke drm_plane_helper_funcs.end_fb_access before
> drm_atomic_helper_commit_hw_done(). The latter function hands over
> ownership of the plane state to the following commit, which might
> free it. Releasing resources in end_fb_access then operates on undefined
> state. This bug has been observed with non-blocking commits when they
> are being queued up quickly.
>
> Here is an example stack trace from the bug report. The plane state has
> been free'd already, so the pages for drm_gem_fb_vunmap() are gone.
>
> Unable to handle kernel paging request at virtual address 00010049
> [...]
> drm_gem_fb_vunmap+0x18/0x74
> drm_gem_end_shadow_fb_access+0x1c/0x2c
> drm_atomic_helper_cleanup_planes+0x58/0xd8
> drm_atomic_helper_commit_tail+0x90/0xa0
> commit_tail+0x15c/0x188
> commit_work+0x14/0x20
>
> Fix this by running end_fb_access immediately after updating all planes
> in drm_atomic_helper_commit_planes(). The existing clean-up helper
> drm_atomic_helper_cleanup_planes() now only handles cleanup_fb.
>
> For aborted commits, roll back from drm_atomic_helper_prepare_planes()
> in the new helper drm_atomic_helper_unprepare_planes(). This case is
> different from regular cleanup, as we have to release the new state;
> regular cleanup releases the old state. The new helper also invokes
> cleanup_fb for all planes.
>
> The changes mostly involve DRM's atomic helpers. Only two drivers, i915
> and nouveau, implement their own commit function. Update them to invoke
> drm_atomic_helper_unprepare_planes(). Drivers with custom commit_tail
> function do not require changes.
>
> v3:
> * add drm_atomic_helper_unprepare_planes() for rolling back
> * use correct state for end_fb_access
> v2:
> * fix test in drm_atomic_helper_cleanup_planes()
>
> Reported-by: Alyssa Ross
> Closes: https://lore.kernel.org/dri-devel/87leazm0ya@alyssa.is/
> Suggested-by: Daniel Vetter
> Fixes: 94d879eaf7fb ("drm/atomic-helper: Add {begin,end}_fb_access to plane
> helpers")
> Signed-off-by: Thomas Zimmermann
> Cc: # v6.2+
I've been running this for days now, and haven't had a single Oops.
Given the rate with which I encountered them before in this
configuration, it looks very likely that the issue is resolved.
Tested-by: Alyssa Ross
And, once the wrong parameter name in the kerneldoc identified by the
kernel test robot is resolved,
Reviewed-by: Alyssa Ross
signature.asc
Description: PGP signature
Re: Radeon regression in 6.6 kernel
Alex Deucher writes: > Phillip, > > Can you test this patch? I was not able to repro the issue on the > navi2x card I had handy, but I think it should fix it. I pulled -rc4 and it still had the problem. I applied this patch, and yes, it fixed it!
Re: [Freedreno] [PATCH] drm: improve the documentation of connector hpd ops
On Sun, 3 Dec 2023 at 16:24, Laurent Pinchart
wrote:
>
> Hi Abhinav,
>
> Thank you for the patch (and thank to Dmitry for pinging me on IRC, this
> patch got burried in my inbox).
>
> On Wed, Sep 20, 2023 at 01:13:58PM -0700, Abhinav Kumar wrote:
> > While making the changes in [1], it was noted that the documentation
> > of the enable_hpd() and disable_hpd() does not make it clear that
> > these ops should not try to do hpd state maintenance and should only
> > attempt to enable/disable hpd related hardware for the connector.
>
> s/attempt to //
I can probably fix this while applying the patch.
>
> >
> > The state management of these calls to make sure these calls are
> > balanced is handled by the DRM core and we should keep it that way
> > to minimize the overhead in the drivers which implement these ops.
> >
> > [1]: https://patchwork.freedesktop.org/patch/558387/
> >
>
> You could add a
>
> Suggested-by: Laurent Pinchart
>
> > Signed-off-by: Abhinav Kumar
>
> Reviewed-by: Laurent Pinchart
>
> > ---
> > include/drm/drm_modeset_helper_vtables.h | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/include/drm/drm_modeset_helper_vtables.h
> > b/include/drm/drm_modeset_helper_vtables.h
> > index e3c3ac615909..a33cf7488737 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -1154,6 +1154,11 @@ struct drm_connector_helper_funcs {
> >* This operation is optional.
> >*
> >* This callback is used by the drm_kms_helper_poll_enable() helpers.
> > + *
> > + * This operation does not need to perform any hpd state tracking as
> > + * the DRM core handles that maintenance and ensures the calls to
> > enable
> > + * and disable hpd are balanced.
> > + *
> >*/
> > void (*enable_hpd)(struct drm_connector *connector);
> >
> > @@ -1165,6 +1170,11 @@ struct drm_connector_helper_funcs {
> >* This operation is optional.
> >*
> >* This callback is used by the drm_kms_helper_poll_disable() helpers.
> > + *
> > + * This operation does not need to perform any hpd state tracking as
> > + * the DRM core handles that maintenance and ensures the calls to
> > enable
> > + * and disable hpd are balanced.
> > + *
> >*/
> > void (*disable_hpd)(struct drm_connector *connector);
> > };
>
> --
> Regards,
>
> Laurent Pinchart
--
With best wishes
Dmitry
Re: (subset) [PATCH RFC v7 00/10] Support for Solid Fill Planes
On Sun, 3 Dec 2023 at 14:15, Simon Ser wrote: > > On Saturday, December 2nd, 2023 at 22:41, Dmitry Baryshkov > wrote: > > > On Fri, 27 Oct 2023 15:32:50 -0700, Jessica Zhang wrote: > > > > > Some drivers support hardware that have optimizations for solid fill > > > planes. This series aims to expose these capabilities to userspace as > > > some compositors have a solid fill flag (ex. SOLID_COLOR in the Android > > > hardware composer HAL) that can be set by apps like the Android Gears > > > test app. > > > > > > In order to expose this capability to userspace, this series will: > > > > > > [...] > > > > > > Applied to drm-misc-next, thanks! > > Where are the IGT and userspace for this uAPI addition? Indeed. I checked that there are uABI acks/reviews, but I didn't check these requirements. Frankly speaking, I thought that they were handled already, before giving the ack. How should we proceed? Should I land a revert? -- With best wishes Dmitry
Re: [PATCH v3 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller
On 30/11/2023 15:16, Dario Binacchi wrote: > From: Michael Trimarchi > > Add documentation for "ilitek,ili9805" panel. > > Signed-off-by: Michael Trimarchi > Signed-off-by: Dario Binacchi > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 03/10] dt-bindings: display: bridge: tc358775: Add support for tc358765
On 02/12/2023 08:54, Tony Lindgren wrote: > The tc358765 is similar to tc358775 except for the stdby-gpios. In what aspect "except for the stby-gpios"? Does not have them? If so, disallow them for that device (:false). Best regards, Krzysztof
Re: [PATCH v2 02/10] dt-bindings: display: bridge: tc358775: Add data-lanes
On 02/12/2023 08:54, Tony Lindgren wrote: > The device uses a clock lane, and 1 to 4 DSI data lanes. Let's add the > data-lanes property starting at 1 similar to what the other bridge > bindings are doing. > > Let's also drop the data-lanes properties in the example for the DSI host > controller to avoid confusion. The configuration of the DSI host depends > on the controller used and is unrelated to the bridge binding. > > Signed-off-by: Tony Lindgren > --- > .../display/bridge/toshiba,tc358775.yaml | 21 --- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml > b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml > --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml > @@ -46,11 +46,26 @@ properties: > > properties: >port@0: > -$ref: /schemas/graph.yaml#/properties/port > +$ref: /schemas/graph.yaml#/$defs/port-base You need to add here additionalProperties: false Best regards, Krzysztof
Re: [PATCH v2 01/10] dt-bindings: display: bridge: tc358775: make stby gpio and vdd supplies optional
On 02/12/2023 08:54, Tony Lindgren wrote: > From: Michael Walle > > For a normal operation, the vdd supplies nor the stby GPIO is needed. > There are boards, where these voltages are statically enabled during > board power-up. This means supply is still required. > > The reset pin is required because once the PPI (PHY protocol interface) > is started, it can only be stopped by asserting the reset pin. > > Signed-off-by: Michael Walle > Signed-off-by: Tony Lindgren > --- Best regards, Krzysztof
Re: [PATCH v2 2/5] ARM: dts: rockchip: Add power-controller for RK3128
Hi Alex,
Am Sonntag, 3. Dezember 2023, 17:05:47 CET schrieb Alex Bee:
> Am 02.12.23 um 18:46 schrieb Heiko Stübner:
> > Am Samstag, 2. Dezember 2023, 17:36:15 CET schrieb Alex Bee:
> >> Am 02.12.23 um 16:51 schrieb Heiko Stübner:
> >>> Am Samstag, 2. Dezember 2023, 13:51:41 CET schrieb Alex Bee:
> Add power controller and qos nodes for RK3128 in order to use
> them as powerdomains.
> >>> does the power-domain controller work with the incomplete set of
> >>> pm-domains too?
> >> Yes, it does - the missing domains can request idle only and can't be
> >> powered on/off - if no one requests idle they are just up all the time.
> >>
> >>> What I have in mind is
> >>> - adding the power-controller node with the existing set of power-domains
> >>> - the gpu pm-domain is in there
> >>> - adding the gpu parts
> >> My main concern about adding them later was the change of the ABI after
> >> they've been exposed in the SoC DT. If that's not an issue - sure: I can
> >> add them in a separate series.
> > An ABI change would be _changing_ the domain-ids in the rk3128-power.h
> > I think :-) .
> Well, an addition is still a change.
> > Right now the existing domain ids in the header are already exposed to the
> > world, so someone could already use them, but not the new ones.
>
> I'm fully aware that nothing would ever hard fail anywhere if the new
> domain ids get added later.
>
> Nevertheless we start using here an ABI which is known to be incomplete.
> For no reason, as the patches (which I am now asked to remove from this
> series) for completion are already there (here).
>
> Anyway, if you prefer it this way: I'm pleased to do so.
I was more thinking of accelerating the gpu-part of the series, as that
really is just waiting for the power-domain node that already has driver
support and domain-ids present.
It looks like you're feeling more strongly about that though, so I'll
definitly not pressure you ;-) .
But I guess the split into IDs and driver change should still be
done, especially as the dt-binding-header likely will want an Ack
from the DT maintainers.
And the power-domain change will go through the new pmdomain
subsystem.
Heiko
> >>> And a second series with
> >>> - patch1 from here
> >>> - a dts patch adding the additional pm-domains to rk3128.dtsi
> >>> - I guess patch1 also should be split into a patch adding the binding-ids
> >>> and a separate patch for the code addition.
> >> Yeah, I noticed this also :)
> >>
> >> Regards,
> >>
> >> Alex
> >>
> >>>
> >>> Heiko
> >>>
> Signed-off-by: Alex Bee
> ---
> arch/arm/boot/dts/rockchip/rk3128.dtsi | 101 +
> 1 file changed, 101 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rockchip/rk3128.dtsi
> b/arch/arm/boot/dts/rockchip/rk3128.dtsi
> index 4e8b38604ecd..b72905db04f7 100644
> --- a/arch/arm/boot/dts/rockchip/rk3128.dtsi
> +++ b/arch/arm/boot/dts/rockchip/rk3128.dtsi
> @@ -8,6 +8,7 @@
> #include
> #include
> #include
> +#include
>
> / {
> compatible = "rockchip,rk3128";
> @@ -133,6 +134,106 @@ smp-sram@0 {
> pmu: syscon@100a {
> compatible = "rockchip,rk3128-pmu", "syscon",
> "simple-mfd";
> reg = <0x100a 0x1000>;
> +
> +power: power-controller {
> +compatible = "rockchip,rk3128-power-controller";
> +#power-domain-cells = <1>;
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +power-domain@RK3128_PD_VIO {
> +reg = ;
> +clocks = < ACLK_CIF>,
> + < HCLK_CIF>,
> + < DCLK_EBC>,
> + < HCLK_EBC>,
> + < ACLK_IEP>,
> + < HCLK_IEP>,
> + < ACLK_LCDC0>,
> + < HCLK_LCDC0>,
> + < PCLK_MIPI>,
> + < ACLK_RGA>,
> + < HCLK_RGA>,
> + < ACLK_VIO0>,
> + < ACLK_VIO1>,
> + < HCLK_VIO>,
> + < HCLK_VIO_H2P>,
> + < DCLK_VOP>,
> + < SCLK_VOP>;
> +pm_qos = <_ebc>,
> + <_iep>,
> +
Re: [PATCH v2 2/5] ARM: dts: rockchip: Add power-controller for RK3128
Hi Heiko,
Am 02.12.23 um 18:46 schrieb Heiko Stübner:
Hi Alex,
Am Samstag, 2. Dezember 2023, 17:36:15 CET schrieb Alex Bee:
Am 02.12.23 um 16:51 schrieb Heiko Stübner:
Am Samstag, 2. Dezember 2023, 13:51:41 CET schrieb Alex Bee:
Add power controller and qos nodes for RK3128 in order to use
them as powerdomains.
does the power-domain controller work with the incomplete set of
pm-domains too?
Yes, it does - the missing domains can request idle only and can't be
powered on/off - if no one requests idle they are just up all the time.
What I have in mind is
- adding the power-controller node with the existing set of power-domains
- the gpu pm-domain is in there
- adding the gpu parts
My main concern about adding them later was the change of the ABI after
they've been exposed in the SoC DT. If that's not an issue - sure: I can
add them in a separate series.
An ABI change would be _changing_ the domain-ids in the rk3128-power.h
I think :-) .
Well, an addition is still a change.
Right now the existing domain ids in the header are already exposed to the
world, so someone could already use them, but not the new ones.
I'm fully aware that nothing would ever hard fail anywhere if the new
domain ids get added later.
Nevertheless we start using here an ABI which is known to be incomplete.
For no reason, as the patches (which I am now asked to remove from this
series) for completion are already there (here).
Anyway, if you prefer it this way: I'm pleased to do so.
Alex
Heiko
And a second series with
- patch1 from here
- a dts patch adding the additional pm-domains to rk3128.dtsi
- I guess patch1 also should be split into a patch adding the binding-ids
and a separate patch for the code addition.
Yeah, I noticed this also :)
Regards,
Alex
Heiko
Signed-off-by: Alex Bee
---
arch/arm/boot/dts/rockchip/rk3128.dtsi | 101 +
1 file changed, 101 insertions(+)
diff --git a/arch/arm/boot/dts/rockchip/rk3128.dtsi
b/arch/arm/boot/dts/rockchip/rk3128.dtsi
index 4e8b38604ecd..b72905db04f7 100644
--- a/arch/arm/boot/dts/rockchip/rk3128.dtsi
+++ b/arch/arm/boot/dts/rockchip/rk3128.dtsi
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
/ {
compatible = "rockchip,rk3128";
@@ -133,6 +134,106 @@ smp-sram@0 {
pmu: syscon@100a {
compatible = "rockchip,rk3128-pmu", "syscon", "simple-mfd";
reg = <0x100a 0x1000>;
+
+ power: power-controller {
+ compatible = "rockchip,rk3128-power-controller";
+ #power-domain-cells = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ power-domain@RK3128_PD_VIO {
+ reg = ;
+ clocks = < ACLK_CIF>,
+< HCLK_CIF>,
+< DCLK_EBC>,
+< HCLK_EBC>,
+< ACLK_IEP>,
+< HCLK_IEP>,
+< ACLK_LCDC0>,
+< HCLK_LCDC0>,
+< PCLK_MIPI>,
+< ACLK_RGA>,
+< HCLK_RGA>,
+< ACLK_VIO0>,
+< ACLK_VIO1>,
+< HCLK_VIO>,
+< HCLK_VIO_H2P>,
+< DCLK_VOP>,
+< SCLK_VOP>;
+ pm_qos = <_ebc>,
+<_iep>,
+<_lcdc>,
+<_rga>,
+<_vip>;
+ #power-domain-cells = <0>;
+ };
+
+ power-domain@RK3128_PD_VIDEO {
+ reg = ;
+ clocks = < ACLK_VDPU>,
+< HCLK_VDPU>,
+< ACLK_VEPU>,
+< HCLK_VEPU>,
+< SCLK_HEVC_CORE>;
+ pm_qos = <_vpu>;
+ #power-domain-cells = <0>;
+ };
+
+ power-domain@RK3128_PD_GPU {
+ reg = ;
+ clocks = < ACLK_GPU>;
+ pm_qos = <_gpu>;
+ #power-domain-cells = <0>;
+ };
+
+ power-domain@RK3128_PD_CRYPTO {
+ reg = ;
+
Re: [PATCH] drm: improve the documentation of connector hpd ops
Hi Abhinav,
Thank you for the patch (and thank to Dmitry for pinging me on IRC, this
patch got burried in my inbox).
On Wed, Sep 20, 2023 at 01:13:58PM -0700, Abhinav Kumar wrote:
> While making the changes in [1], it was noted that the documentation
> of the enable_hpd() and disable_hpd() does not make it clear that
> these ops should not try to do hpd state maintenance and should only
> attempt to enable/disable hpd related hardware for the connector.
s/attempt to //
>
> The state management of these calls to make sure these calls are
> balanced is handled by the DRM core and we should keep it that way
> to minimize the overhead in the drivers which implement these ops.
>
> [1]: https://patchwork.freedesktop.org/patch/558387/
>
You could add a
Suggested-by: Laurent Pinchart
> Signed-off-by: Abhinav Kumar
Reviewed-by: Laurent Pinchart
> ---
> include/drm/drm_modeset_helper_vtables.h | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/drm/drm_modeset_helper_vtables.h
> b/include/drm/drm_modeset_helper_vtables.h
> index e3c3ac615909..a33cf7488737 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -1154,6 +1154,11 @@ struct drm_connector_helper_funcs {
>* This operation is optional.
>*
>* This callback is used by the drm_kms_helper_poll_enable() helpers.
> + *
> + * This operation does not need to perform any hpd state tracking as
> + * the DRM core handles that maintenance and ensures the calls to enable
> + * and disable hpd are balanced.
> + *
>*/
> void (*enable_hpd)(struct drm_connector *connector);
>
> @@ -1165,6 +1170,11 @@ struct drm_connector_helper_funcs {
>* This operation is optional.
>*
>* This callback is used by the drm_kms_helper_poll_disable() helpers.
> + *
> + * This operation does not need to perform any hpd state tracking as
> + * the DRM core handles that maintenance and ensures the calls to enable
> + * and disable hpd are balanced.
> + *
>*/
> void (*disable_hpd)(struct drm_connector *connector);
> };
--
Regards,
Laurent Pinchart
[PATCH v3 7/9] drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gt/uc/intel_us_fw.c, the function intel_uc_fw_copy_rsa()
just use the mapping to do memory copy so it doesn't need to disable
pagefaults and preemption for mapping. Thus the local mapping without
atomic context (not disable pagefaults / preemption) is enough.
Therefore, intel_uc_fw_copy_rsa() is a function where the use of
memcpy_from_page() with kmap_local_page() in place of memcpy() with
kmap_atomic() is correctly suited.
Convert the calls of memcpy() with kmap_atomic() / kunmap_atomic() to
memcpy_from_page() which uses local mapping to copy.
[1]:
https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com/T/#u
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Ira: Referred to his task document and suggestions about using
memcpy_from_page() directly.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 362639162ed6..756093eaf2ad 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -1343,16 +1343,13 @@ size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw,
void *dst, u32 max_len)
for_each_sgt_page(page, iter, uc_fw->obj->mm.pages) {
u32 len = min_t(u32, size, PAGE_SIZE - offset);
- void *vaddr;
if (idx > 0) {
idx--;
continue;
}
- vaddr = kmap_atomic(page);
- memcpy(dst, vaddr + offset, len);
- kunmap_atomic(vaddr);
+ memcpy_from_page(dst, page, offset, len);
offset = 0;
dst += len;
--
2.34.1
[PATCH v3 9/9] drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the calls from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
In i915_gem_execbuffer.c, eb->reloc_cache.vaddr is mapped by
kmap_atomic() in eb_relocate_entry(), and is unmapped by
kunmap_atomic() in reloc_cache_reset().
And this mapping/unmapping occurs in two places: one is in
eb_relocate_vma(), and another is in eb_relocate_vma_slow().
The function eb_relocate_vma() or eb_relocate_vma_slow() doesn't
need to disable pagefaults and preemption during the above mapping/
unmapping.
So it can simply use kmap_local_page() / kunmap_local() that can
instead do the mapping / unmapping regardless of the context.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Ira: Referred to his task document, review comments.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 683fd8d3151c..18b0f3117074 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1156,7 +1156,7 @@ static void reloc_cache_unmap(struct reloc_cache *cache)
vaddr = unmask_page(cache->vaddr);
if (cache->vaddr & KMAP)
- kunmap_atomic(vaddr);
+ kunmap_local(vaddr);
else
io_mapping_unmap_atomic((void __iomem *)vaddr);
}
@@ -1172,7 +1172,7 @@ static void reloc_cache_remap(struct reloc_cache *cache,
if (cache->vaddr & KMAP) {
struct page *page = i915_gem_object_get_page(obj, cache->page);
- vaddr = kmap_atomic(page);
+ vaddr = kmap_local_page(page);
cache->vaddr = unmask_flags(cache->vaddr) |
(unsigned long)vaddr;
} else {
@@ -1202,7 +1202,7 @@ static void reloc_cache_reset(struct reloc_cache *cache,
struct i915_execbuffer
if (cache->vaddr & CLFLUSH_AFTER)
mb();
- kunmap_atomic(vaddr);
+ kunmap_local(vaddr);
i915_gem_object_finish_access(obj);
} else {
struct i915_ggtt *ggtt = cache_to_ggtt(cache);
@@ -1234,7 +1234,7 @@ static void *reloc_kmap(struct drm_i915_gem_object *obj,
struct page *page;
if (cache->vaddr) {
- kunmap_atomic(unmask_page(cache->vaddr));
+ kunmap_local(unmask_page(cache->vaddr));
} else {
unsigned int flushes;
int err;
@@ -1256,7 +1256,7 @@ static void *reloc_kmap(struct drm_i915_gem_object *obj,
if (!obj->mm.dirty)
set_page_dirty(page);
- vaddr = kmap_atomic(page);
+ vaddr = kmap_local_page(page);
cache->vaddr = unmask_flags(cache->vaddr) | (unsigned long)vaddr;
cache->page = pageno;
--
2.34.1
[PATCH v3 2/9] drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() + memcpy() to memcpy_[from/to]_page(), which use
kmap_local_page() to build local mapping and then do memcpy().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
In drm/i915/gem/i915_gem_phys.c, the functions
i915_gem_object_get_pages_phys() and i915_gem_object_put_pages_phys()
don't need to disable pagefaults and preemption for mapping because of
2 reasons:
1. The flush operation is safe. In drm/i915/gem/i915_gem_object.c,
i915_gem_object_get_pages_phys() and i915_gem_object_put_pages_phys()
calls drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush.
Since CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, i915_gem_object_get_pages_phys() and
i915_gem_object_put_pages_phys() are two functions where the uses of
local mappings in place of atomic mappings are correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() + memcpy() to
memcpy_from_page() and memcpy_to_page().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 5df128e2f4dc..ef85c6dc9fd5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -65,16 +65,13 @@ static int i915_gem_object_get_pages_phys(struct
drm_i915_gem_object *obj)
dst = vaddr;
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
struct page *page;
- void *src;
page = shmem_read_mapping_page(mapping, i);
if (IS_ERR(page))
goto err_st;
- src = kmap_atomic(page);
- memcpy(dst, src, PAGE_SIZE);
+ memcpy_from_page(dst, page, 0, PAGE_SIZE);
drm_clflush_virt_range(dst, PAGE_SIZE);
- kunmap_atomic(src);
put_page(page);
dst += PAGE_SIZE;
@@ -113,16 +110,13 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object
*obj,
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
struct page *page;
- char *dst;
page = shmem_read_mapping_page(mapping, i);
if (IS_ERR(page))
continue;
- dst = kmap_atomic(page);
drm_clflush_virt_range(src, PAGE_SIZE);
- memcpy(dst, src, PAGE_SIZE);
- kunmap_atomic(dst);
+ memcpy_to_page(page, 0, src, PAGE_SIZE);
set_page_dirty(page);
if (obj->mm.madv == I915_MADV_WILLNEED)
--
2.34.1
[PATCH v3 3/9] drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c
From: Zhao Liu The use of kmap_atomic() is being deprecated in favor of kmap_local_page()[1]. The main difference between atomic and local mappings is that local mappings doesn't disable page faults or preemption (the preemption is disabled for !PREEMPT_RT case, otherwise it only disables migration). With kmap_local_page(), we can avoid the often unwanted side effect of unnecessary page faults or preemption disables. In drm/i915/gem/i915_gem_shmem.c, the function shmem_pwrite() need to disable pagefault to eliminate the potential recursion fault[2]. But here __copy_from_user_inatomic() doesn't need to disable preemption and local mapping is valid for sched in/out. So it can use kmap_local_page() / kunmap_local() with pagefault_disable() / pagefault_enable() to replace atomic mapping. [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com [2]: https://patchwork.freedesktop.org/patch/295840/ Suggested-by: Ira Weiny Suggested-by: Fabio M. De Francesco Signed-off-by: Zhao Liu Reviewed-by: Ira Weiny Reviewed-by: Fabio M. De Francesco --- Suggested by credits: Ira: Referred to his suggestions about keeping pagefault_disable(). Fabio: Referred to his description about why kmap_local_page() should be preferred. --- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 73a4a4eb29e0..38b72d86560f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -485,11 +485,13 @@ shmem_pwrite(struct drm_i915_gem_object *obj, if (err < 0) return err; - vaddr = kmap_atomic(page); + vaddr = kmap_local_page(page); + pagefault_disable(); unwritten = __copy_from_user_inatomic(vaddr + pg, user_data, len); - kunmap_atomic(vaddr); + pagefault_enable(); + kunmap_local(vaddr); err = aops->write_end(obj->base.filp, mapping, offset, len, len - unwritten, page, data); -- 2.34.1
[PATCH v3 8/9] drm/i915: Use kmap_local_page() in i915_cmd_parser.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
There're 2 reasons why function copy_batch() doesn't need to disable
pagefaults and preemption for mapping:
1. The flush operation is safe. In i915_cmd_parser.c, copy_batch() calls
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush.
Since CLFLUSHOPT is global on x86 and WBINVD is called on each cpu
in drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, copy_batch() is a function where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index ddf49c2dbb91..2905df83e180 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1211,11 +1211,11 @@ static u32 *copy_batch(struct drm_i915_gem_object
*dst_obj,
for (n = offset >> PAGE_SHIFT; remain; n++) {
int len = min(remain, PAGE_SIZE - x);
- src = kmap_atomic(i915_gem_object_get_page(src_obj, n));
+ src = kmap_local_page(i915_gem_object_get_page(src_obj,
n));
if (src_needs_clflush)
drm_clflush_virt_range(src + x, len);
memcpy(ptr, src + x, len);
- kunmap_atomic(src);
+ kunmap_local(src);
ptr += len;
remain -= len;
--
2.34.1
[PATCH v3 5/9] drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration)..
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/selftests/i915_gem_coherency.c, functions cpu_set()
and cpu_get() mainly uses mapping to flush cache and assign the value.
There're 2 reasons why cpu_set() and cpu_get() don't need to disable
pagefaults and preemption for mapping:
1. The flush operation is safe. cpu_set() and cpu_get() call
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush. Since
CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, cpu_set() and cpu_get() are functions where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
.../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
index 3bef1beec7cb..beeb3e12eccc 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c
@@ -24,7 +24,6 @@ static int cpu_set(struct context *ctx, unsigned long offset,
u32 v)
{
unsigned int needs_clflush;
struct page *page;
- void *map;
u32 *cpu;
int err;
@@ -34,8 +33,7 @@ static int cpu_set(struct context *ctx, unsigned long offset,
u32 v)
goto out;
page = i915_gem_object_get_page(ctx->obj, offset >> PAGE_SHIFT);
- map = kmap_atomic(page);
- cpu = map + offset_in_page(offset);
+ cpu = kmap_local_page(page) + offset_in_page(offset);
if (needs_clflush & CLFLUSH_BEFORE)
drm_clflush_virt_range(cpu, sizeof(*cpu));
@@ -45,7 +43,7 @@ static int cpu_set(struct context *ctx, unsigned long offset,
u32 v)
if (needs_clflush & CLFLUSH_AFTER)
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap_atomic(map);
+ kunmap_local(cpu);
i915_gem_object_finish_access(ctx->obj);
out:
@@ -57,7 +55,6 @@ static int cpu_get(struct context *ctx, unsigned long offset,
u32 *v)
{
unsigned int needs_clflush;
struct page *page;
- void *map;
u32 *cpu;
int err;
@@ -67,15 +64,14 @@ static int cpu_get(struct context *ctx, unsigned long
offset, u32 *v)
goto out;
page = i915_gem_object_get_page(ctx->obj, offset >> PAGE_SHIFT);
- map = kmap_atomic(page);
- cpu = map + offset_in_page(offset);
+ cpu = kmap_local_page(page) + offset_in_page(offset);
if (needs_clflush & CLFLUSH_BEFORE)
drm_clflush_virt_range(cpu, sizeof(*cpu));
*v = *cpu;
- kunmap_atomic(map);
+ kunmap_local(cpu);
i915_gem_object_finish_access(ctx->obj);
out:
--
2.34.1
[PATCH v3 6/9] drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption.
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/selftests/i915_gem_context.c, functions cpu_fill() and
cpu_check() mainly uses mapping to flush cache and check/assign the
value.
There're 2 reasons why cpu_fill() and cpu_check() don't need to disable
pagefaults and preemption for mapping:
1. The flush operation is safe. cpu_fill() and cpu_check() call
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush. Since
CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, cpu_fill() and cpu_check() are functions where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 7021b6e9b219..89d4dc8b60c6 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -489,12 +489,12 @@ static int cpu_fill(struct drm_i915_gem_object *obj, u32
value)
for (n = 0; n < real_page_count(obj); n++) {
u32 *map;
- map = kmap_atomic(i915_gem_object_get_page(obj, n));
+ map = kmap_local_page(i915_gem_object_get_page(obj, n));
for (m = 0; m < DW_PER_PAGE; m++)
map[m] = value;
if (!has_llc)
drm_clflush_virt_range(map, PAGE_SIZE);
- kunmap_atomic(map);
+ kunmap_local(map);
}
i915_gem_object_finish_access(obj);
@@ -520,7 +520,7 @@ static noinline int cpu_check(struct drm_i915_gem_object
*obj,
for (n = 0; n < real_page_count(obj); n++) {
u32 *map, m;
- map = kmap_atomic(i915_gem_object_get_page(obj, n));
+ map = kmap_local_page(i915_gem_object_get_page(obj, n));
if (needs_flush & CLFLUSH_BEFORE)
drm_clflush_virt_range(map, PAGE_SIZE);
@@ -546,7 +546,7 @@ static noinline int cpu_check(struct drm_i915_gem_object
*obj,
}
out_unmap:
- kunmap_atomic(map);
+ kunmap_local(map);
if (err)
break;
}
--
2.34.1
[PATCH v3 4/9] drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults or preemption disables.
In drm/i915/gem/selftests/huge_pages.c, function __cpu_check_shmem()
mainly uses mapping to flush cache and check the value. There're
2 reasons why __cpu_check_shmem() doesn't need to disable pagefaults
and preemption for mapping:
1. The flush operation is safe. Function __cpu_check_shmem() calls
drm_clflush_virt_range() to use CLFLUSHOPT or WBINVD to flush. Since
CLFLUSHOPT is global on x86 and WBINVD is called on each cpu in
drm_clflush_virt_range(), the flush operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, __cpu_check_shmem() is a function where the use of
kmap_local_page() in place of kmap_atomic() is correctly suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 6b9f6cf50bf6..c9e6d77abab0 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1082,7 +1082,7 @@ __cpu_check_shmem(struct drm_i915_gem_object *obj, u32
dword, u32 val)
goto err_unlock;
for (n = 0; n < obj->base.size >> PAGE_SHIFT; ++n) {
- u32 *ptr = kmap_atomic(i915_gem_object_get_page(obj, n));
+ u32 *ptr = kmap_local_page(i915_gem_object_get_page(obj, n));
if (needs_flush & CLFLUSH_BEFORE)
drm_clflush_virt_range(ptr, PAGE_SIZE);
@@ -1090,12 +1090,12 @@ __cpu_check_shmem(struct drm_i915_gem_object *obj, u32
dword, u32 val)
if (ptr[dword] != val) {
pr_err("n=%lu ptr[%u]=%u, val=%u\n",
n, dword, ptr[dword], val);
- kunmap_atomic(ptr);
+ kunmap_local(ptr);
err = -EINVAL;
break;
}
- kunmap_atomic(ptr);
+ kunmap_local(ptr);
}
i915_gem_object_finish_access(obj);
--
2.34.1
[PATCH v3 1/9] drm/i915: Use kmap_local_page() in gem/i915_gem_object.c
From: Zhao Liu
The use of kmap_atomic() is being deprecated in favor of
kmap_local_page()[1], and this patch converts the call from
kmap_atomic() to kmap_local_page().
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
There're 2 reasons why i915_gem_object_read_from_page_kmap() doesn't
need to disable pagefaults and preemption for mapping:
1. The flush operation is safe. In drm/i915/gem/i915_gem_object.c,
i915_gem_object_read_from_page_kmap() calls drm_clflush_virt_range() to
use CLFLUSHOPT or WBINVD to flush. Since CLFLUSHOPT is global on x86
and WBINVD is called on each cpu in drm_clflush_virt_range(), the flush
operation is global.
2. Any context switch caused by preemption or page faults (page fault
may cause sleep) doesn't affect the validity of local mapping.
Therefore, i915_gem_object_read_from_page_kmap() is a function where
the use of kmap_local_page() in place of kmap_atomic() is correctly
suited.
Convert the calls of kmap_atomic() / kunmap_atomic() to
kmap_local_page() / kunmap_local().
And remove the redundant variable that stores the address of the mapped
page since kunmap_local() can accept any pointer within the page.
[1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Suggested-by: Dave Hansen
Suggested-by: Ira Weiny
Suggested-by: Fabio M. De Francesco
Signed-off-by: Zhao Liu
Reviewed-by: Ira Weiny
Reviewed-by: Fabio M. De Francesco
---
Suggested by credits:
Dave: Referred to his explanation about cache flush.
Ira: Referred to his task document, review comments and explanation
about cache flush.
Fabio: Referred to his boiler plate commit message and his description
about why kmap_local_page() should be preferred.
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index c26d87555825..a2a7e5005415 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -493,17 +493,15 @@ static void
i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
pgoff_t idx = offset >> PAGE_SHIFT;
- void *src_map;
void *src_ptr;
- src_map = kmap_atomic(i915_gem_object_get_page(obj, idx));
-
- src_ptr = src_map + offset_in_page(offset);
+ src_ptr = kmap_local_page(i915_gem_object_get_page(obj, idx))
+ + offset_in_page(offset);
if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
drm_clflush_virt_range(src_ptr, size);
memcpy(dst, src_ptr, size);
- kunmap_atomic(src_map);
+ kunmap_local(src_ptr);
}
static void
--
2.34.1
[PATCH v3 0/9] drm/i915: Replace kmap_atomic() with kmap_local_page()
From: Zhao Liu
Hi all,
I refreshed this v3 by rebasing v2 [1] on the commit 968f35f4ab1c
("Merge tag 'v6.7-rc3-smb3-client-fixes' of git://git.samba.org/sfrench/
cifs-2.6").
Based on the current code, I rechecked the substitutions in v2 and they
still stand and are valid, so no code change in v3.
Thanks for all the review! And sorry v2 was missed, I'll pay more
attention to this v3.
Purpose of This Patchset
The purpose of this pacthset is to replace all uses of kmap_atomic() in
i915 with kmap_local_page() because the use of kmap_atomic() is being
deprecated in favor of kmap_local_page()[2]. And 92b64bd (mm/highmem:
add notes about conversions from kmap{,_atomic}()) has declared the
deprecation of kmap_atomic().
Motivation for Deprecating kmap_atomic() and Using kmap_local_page()
The main difference between atomic and local mappings is that local
mappings doesn't disable page faults or preemption (the preemption is
disabled for !PREEMPT_RT case, otherwise it only disables migration).
With kmap_local_page(), we can avoid the often unwanted side effect of
unnecessary page faults and preemption disables.
Patch summary
=
Patch 1, 4-6 and 8-9 replace kmap_atomic()/kunmap_atomic() with
kmap_local_page()/kunmap_local() directly. With these local
mappings, the page faults and preemption are allowed.
Patch 2 and 7 use memcpy_from_page() and memcpy_to_page() to replace
kmap_atomic()/kunmap_atomic(). These two variants of memcpy()
are based on the local mapping, so page faults and preemption
are also allowed in these two interfaces.
Patch 3 replaces kmap_atomic()/kunmap_atomic() with kmap_local_page()/
kunmap_local() and also disable page fault since the for special
handling (pls see the commit message).
Reference
=
[1]:
https://lore.kernel.org/all/20230329073220.3982460-1-zhao1@linux.intel.com/
[2]: https://lore.kernel.org/all/20220813220034.806698-1-ira.we...@intel.com
Thanks and Best Regards,
Zhao
---
Changlog:
Changes since v2:
* Rebased on 968f35f4ab1c ("Merge tag 'v6.7-rc3-smb3-client-fixes' of
git://git.samba.org/sfrench/cifs-2.6").
* Removed changelog (of v2) in commit message.
* Fixed typo in cover letter (Fabio).
* Added Reviewed-by tags from Ira and Fabio.
Changes since v1:
* Dropped hot plug related description in commit message since it has
nothing to do with kmap_local_page().
* Emphasized the motivation for using kmap_local_page() in commit
message.
* Rebased patch 1 on f47e630 (drm/i915/gem: Typecheck page lookups) to
keep the "idx" variable of type pgoff_t here.
* Used memcpy_from_page() and memcpy_to_page() to replace
kmap_local_page() + memcpy() in patch 2.
---
Zhao Liu (9):
drm/i915: Use kmap_local_page() in gem/i915_gem_object.c
drm/i915: Use memcpy_[from/to]_page() in gem/i915_gem_pyhs.c
drm/i915: Use kmap_local_page() in gem/i915_gem_shmem.c
drm/i915: Use kmap_local_page() in gem/selftests/huge_pages.c
drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_coherency.c
drm/i915: Use kmap_local_page() in gem/selftests/i915_gem_context.c
drm/i915: Use memcpy_from_page() in gt/uc/intel_uc_fw.c
drm/i915: Use kmap_local_page() in i915_cmd_parser.c
drm/i915: Use kmap_local_page() in gem/i915_gem_execbuffer.c
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 10 +-
drivers/gpu/drm/i915/gem/i915_gem_object.c | 8 +++-
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 10 ++
drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 6 --
drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 6 +++---
.../gpu/drm/i915/gem/selftests/i915_gem_coherency.c | 12
.../gpu/drm/i915/gem/selftests/i915_gem_context.c| 8
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 +
drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
9 files changed, 28 insertions(+), 41 deletions(-)
--
2.34.1
Re: [RFC PATCH 2/6] mm/gmem: add arch-independent abstraction to track address mapping status
On Fri, Dec 1, 2023 at 9:23 AM David Hildenbrand wrote: > > On 28.11.23 13:50, Weixi Zhu wrote: > > This patch adds an abstraction layer, struct vm_object, that maintains > > per-process virtual-to-physical mapping status stored in struct gm_mapping. > > For example, a virtual page may be mapped to a CPU physical page or to a > > device physical page. Struct vm_object effectively maintains an > > arch-independent page table, which is defined as a "logical page table". > > While arch-dependent page table used by a real MMU is named a "physical > > page table". The logical page table is useful if Linux core MM is extended > > to handle a unified virtual address space with external accelerators using > > customized MMUs. > > Which raises the question why we are dealing with anonymous memory at > all? Why not go for shmem if you are already only special-casing VMAs > with a MMAP flag right now? > > That would maybe avoid having to introduce controversial BSD design > concepts into Linux, that feel like going a step backwards in time to me > and adding *more* MM complexity. > > > > > In this patch, struct vm_object utilizes a radix > > tree (xarray) to track where a virtual page is mapped to. This adds extra > > memory consumption from xarray, but provides a nice abstraction to isolate > > mapping status from the machine-dependent layer (PTEs). Besides supporting > > accelerators with external MMUs, struct vm_object is planned to further > > union with i_pages in struct address_mapping for file-backed memory. > > A file already has a tree structure (pagecache) to manage the pages that > are theoretically mapped. It's easy to translate from a VMA to a page > inside that tree structure that is currently not present in page tables. > > Why the need for that tree structure if you can just remove anon memory > from the picture? > > > > > The idea of struct vm_object is originated from FreeBSD VM design, which > > provides a unified abstraction for anonymous memory, file-backed memory, > > page cache and etc[1]. > > :/ > > > Currently, Linux utilizes a set of hierarchical page walk functions to > > abstract page table manipulations of different CPU architecture. The > > problem happens when a device wants to reuse Linux MM code to manage its > > page table -- the device page table may not be accessible to the CPU. > > Existing solution like Linux HMM utilizes the MMU notifier mechanisms to > > invoke device-specific MMU functions, but relies on encoding the mapping > > status on the CPU page table entries. This entangles machine-independent > > code with machine-dependent code, and also brings unnecessary restrictions. > > Why? we have primitives to walk arch page tables in a non-arch specific > fashion and are using them all over the place. > > We even have various mechanisms to map something into the page tables > and get the CPU to fault on it, as if it is inaccessible (PROT_NONE as > used for NUMA balancing, fake swap entries). > > > The PTE size and format vary arch by arch, which harms the extensibility. > > Not really. > > We might have some features limited to some architectures because of the > lack of PTE bits. And usually the problem is that people don't care > enough about enabling these features on older architectures. > > If we ever *really* need more space for sw-defined data, it would be > possible to allocate auxiliary data for page tables only where required > (where the features apply), instead of crafting a completely new, > auxiliary datastructure with it's own locking. > > So far it was not required to enable the feature we need on the > architectures we care about. > > > > > [1] https://docs.freebsd.org/en/articles/vm-design/ > > In the cover letter you have: > > "The future plan of logical page table is to provide a generic > abstraction layer that support common anonymous memory (I am looking at > you, transparent huge pages) and file-backed memory." > > Which I doubt will happen; there is little interest in making anonymous > memory management slower, more serialized, and wasting more memory on > metadata. Also worth noting that: 1) Mach VM (which FreeBSD inherited, from the old BSD) vm_objects aren't quite what's being stated here, rather they are somewhat replacements for both anon_vma and address_space[1]. Very similarly to Linux, they take pages from vm_objects and map them in page tables using pmap (the big difference is anon memory, which has its bookkeeping in page tables, on Linux) 2) These vm_objects were a horrendous mistake (see CoW chaining) and FreeBSD has to go to horrendous lengths to make them tolerable. The UVM paper/dissertation (by Charles Cranor) talks about these issues at length, and 20 years later it's still true. 3) Despite Linux MM having its warts, it's probably correct to consider it a solid improvement over FreeBSD MM or NetBSD UVM And, finally, randomly tacking on core MM concepts from other systems is at best a *really weird* idea. Particularly when they
Re: [Nouveau] Thinkpad P17 gen 2 kernel 6.4 and 6.6 lack of support for nvidia GA104GLM [RTX A5000 Mobile] and missing module firmware
On Sat, Dec 02, 2023 at 06:08:01PM +, Timur Tabi wrote: > On Sat, 2023-12-02 at 09:13 -0800, Marc MERLIN wrote: > > [ 3.184525] nouveau: unknown parameter 'modset' ignored > > For starters, you misspelled "modeset" That was a previous boot in dmesg where I failed to turn off the module, but I was mostly interested in showing the errors of all the firmware missing and nouveau failing to start, which those logs do show. Separely, both 6.4 and 6.6 are hanging after a few hours of runtime with networking dying or other issues that require reboot See below 6.4: > [55647.774842] vgaarb: client 0xc24cb19e called 'target' > [55647.774852] vgaarb: PCI::00:02.0 ==> :00:02.0 pdev bfa35d85 > [55647.774854] vgaarb: vgadev 8ea0fc7d > [55825.318992] INFO: task NetworkManager:3372 blocked for more than 120 > seconds. > [55825.318999] Tainted: G U OE > 6.4.9-amd64-preempt-sysrq-20220227 #2 > [55825.319000] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > this message. > [55825.319002] task:NetworkManager state:D stack:0 pid:3372 ppid:1 > flags:0x0002 > [55825.319005] Call Trace: > [55825.319006] > [55825.319009] __schedule+0xba5/0xc17 > [55825.319015] schedule+0x95/0xce > [55825.319017] schedule_preempt_disabled+0x15/0x22 > [55825.319020] __mutex_lock.constprop.0+0x18b/0x291 > [55825.319025] nl80211_prepare_wdev_dump+0x8b/0x19f [cfg80211 > d0c23c84d531afea8d4a2711c5c3e691cbb9587f] > [55825.319065] nl80211_dump_station+0x49/0x1d0 [cfg80211 > d0c23c84d531afea8d4a2711c5c3e691cbb9587f] > [55825.319091] ? __mod_lruvec_page_state+0x4c/0x86 > [55825.319093] ? mod_lruvec_page_state.constprop.0+0x1c/0x2e > [55825.319096] ? __kmalloc_large_node+0xd5/0xfb > [55825.319099] ? __kmalloc_node_track_caller+0x5a/0xad > [55825.319101] ? kmalloc_reserve+0xa7/0xe2 > [55825.319104] ? __alloc_skb+0xe9/0x148 > [55825.319106] netlink_dump+0x143/0x2b2 > [55825.319109] __netlink_dump_start+0x125/0x177 > [55825.319111] genl_family_rcv_msg_dumpit+0xf1/0x110 > [55825.319114] ? poll_freewait+0x72/0x91 > [55825.319117] ? __pfx_genl_start+0x40/0x40 > [55825.319119] ? __pfx_nl80211_dump_station+0x40/0x40 [cfg80211 > d0c23c84d531afea8d4a2711c5c3e691cbb9587f] > [55825.319143] ? __pfx_genl_parallel_done+0x40/0x40 > [55825.319146] genl_rcv_msg+0x189/0x1e2 > [55825.319148] ? __pfx_nl80211_dump_station+0x40/0x40 [cfg80211 > d0c23c84d531afea8d4a2711c5c3e691cbb9587f] > [55825.319172] ? __pfx_genl_rcv_msg+0x40/0x40 > [55825.319173] netlink_rcv_skb+0x89/0xe3 > [55825.319176] genl_rcv+0x24/0x31 > [55825.319178] netlink_unicast+0x10e/0x1ae > [55825.319180] netlink_sendmsg+0x321/0x361 > [55825.319182] sock_sendmsg_nosec+0x35/0x64 > [55825.319186] sys_sendmsg+0x13e/0x1ef > [55825.319188] ___sys_sendmsg+0x76/0xb3 > [55825.319190] ? __fget_light+0x41/0x50 > [55825.319193] ? do_epoll_wait+0x49b/0x4d4 > [55825.319196] ? __pfx_pollwake+0x40/0x40 > [55825.319198] ? __rseq_handle_notify_resume+0x2a0/0x4bd > [55825.319200] ? __fget+0x38/0x47 > [55825.319202] __sys_sendmsg+0x60/0x97 > [55825.319204] do_syscall_64+0x7e/0xa7 > [55825.319208] ? syscall_exit_to_user_mode+0x18/0x27 > [55825.319210] ? __task_pid_nr_ns+0x5f/0x6d > [55825.319213] ? syscall_exit_to_user_mode+0x18/0x27 > [55825.319214] ? do_syscall_64+0x9d/0xa7 > [55825.319216] ? do_syscall_64+0x9d/0xa7 > [55825.319218] ? do_syscall_64+0x9d/0xa7 > [55825.319220] ? do_syscall_64+0x9d/0xa7 > [55825.319222] entry_SYSCALL_64_after_hwframe+0x77/0xe1 > [55825.319224] RIP: 0033:0x7f1fdc79e9bd > [55825.319226] RSP: 002b:7ffeb6460900 EFLAGS: 0293 ORIG_RAX: > 002e > [55825.319228] RAX: ffda RBX: 55e0a9ce1d90 RCX: > 7f1fdc79e9bd > [55825.319229] RDX: RSI: 7ffeb6460950 RDI: > 000b > [55825.319230] RBP: 7ffeb6460950 R08: R09: > 0300 > [55825.319231] R10: R11: 0293 R12: > 7ffeb6460a30 > [55825.319232] R13: 7f1fd0038690 R14: 7ffeb6460c60 R15: > 55e0aa210400 > [55825.319234] 6.6.3: [ 443.613095] BTRFS info (device dm-2): scrub: started on devid 1 [ 484.778344] INFO: task kworker/2:1:106 blocked for more than 120 seconds. [ 484.778352] Tainted: G U 6.6.3-amd64-preempt-sysrq-20220227 #4 [ 484.778353] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 484.778354] task:kworker/2:1 state:D stack:0 pid:106 ppid:2 flags:0x4000 [ 484.778358] Workqueue: ipv6_addrconf addrconf_verify_work [ 484.778365] Call Trace: [ 484.778367] [ 484.778369] __schedule+0xba0/0xc05 [ 484.778373] schedule+0x95/0xce [ 484.778375] schedule_preempt_disabled+0x15/0x22 [ 484.778376] __mutex_lock.constprop.0+0x18b/0x291 [ 484.778379] addrconf_verify_work+0xe/0x20 [ 484.778381] process_scheduled_works+0x1da/0x2e0 [ 484.778385] worker_thread+0x1ca/0x224 [ 484.778388] ?
Thinkpad P17 gen 2 kernel 6.4 and 6.6 lack of support for nvidia GA104GLM [RTX A5000 Mobile] and missing module firmware
Howdy, I'm trying a Thnkpad P17 gen2, the last thinkpad that still comes in 17" 4K (newer ones are 16" only, so I'm looking for other worthwhile linux laptops with 17" or bigger LCD that also does 4K, the alienware I saw was 18" but not 4K) Unfortunately I seem to need the nouveau driver to turn off the nvidia chip I don't plan on using (intel graphics is fine for me), and bios only allows 'bybrid' or nvidia only) On my P73, nouveau never really worked in the 3 years I've had it, but it could at least turn off the nvidia chip. On P17gen2 it does not seem to be able to do so. Firmware is missing even from the latest firmware-linux-nonfree or from upstream git https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git sauron:~# update-initramfs -v -c -k `uname -r` 2>&1 |grep W: W: Possible missing firmware /lib/firmware/nvidia/ga107/acr/ucode_ahesasc.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga106/acr/ucode_ahesasc.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga104/acr/ucode_ahesasc.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga103/acr/ucode_ahesasc.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga107/acr/ucode_asb.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga106/acr/ucode_asb.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga104/acr/ucode_asb.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga103/acr/ucode_asb.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga107/acr/ucode_unload.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga106/acr/ucode_unload.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga104/acr/ucode_unload.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga103/acr/ucode_unload.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga107/nvdec/scrubber.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga106/nvdec/scrubber.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga104/nvdec/scrubber.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga103/nvdec/scrubber.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga107/sec2/hs_bl_sig.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga107/sec2/sig.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga107/sec2/image.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga107/sec2/desc.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga106/sec2/hs_bl_sig.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga106/sec2/sig.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga106/sec2/image.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga106/sec2/desc.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga104/sec2/hs_bl_sig.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga104/sec2/sig.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga104/sec2/image.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga104/sec2/desc.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga103/sec2/hs_bl_sig.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga103/sec2/sig.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga103/sec2/image.bin for module nouveau W: Possible missing firmware /lib/firmware/nvidia/ga103/sec2/desc.bin for module nouveau During boot, the nvidia module hangs for 2mn and fails to do any work, including being able to turn off the nvidia chip (which it could do on P73 without otherwise ever being able to use that chip for proper display). I want to turn off the nvidia chip so that I can get multi hour runtime on batteries without some useless chip that is using power for no reason. sauron:~# lspci | grep VGA 00:02.0 VGA compatible controller: Intel Corporation Tiger Lake-H GT1 [UHD Graphics] (rev 01) 01:00.0 VGA compatible controller: NVIDIA Corporation GA104GLM [RTX A5000 Mobile] (rev a1) Boot looks like this: [0.210932] Kernel command line: BOOT_IMAGE=/vmlinuz-6.6.3-amd64-preempt-sysrq-20220227 root=/dev/mapper/cryptroot ro rootflags=subvol=root cryptopts=source=/dev/nvme0n1p7,keyscript=/sbin/cryptgetpw usbcore.autosuspend=1 pcie_aspm=force resume=/dev/dm-1 thinkpad-acpi.brightness_enable=1 acpi_backlight=native nouveau.modset=0 systemd.unified_cgroup_hierarchy=0 [3.184525] nouveau: unknown parameter 'modset' ignored [3.184800] nouveau: detected PR support, will not use DSM [3.184813] nouveau :01:00.0: vgaarb: pci_notify [3.184816] nouveau :01:00.0:
Re: (subset) [PATCH RFC v7 00/10] Support for Solid Fill Planes
On Saturday, December 2nd, 2023 at 22:41, Dmitry Baryshkov wrote: > On Fri, 27 Oct 2023 15:32:50 -0700, Jessica Zhang wrote: > > > Some drivers support hardware that have optimizations for solid fill > > planes. This series aims to expose these capabilities to userspace as > > some compositors have a solid fill flag (ex. SOLID_COLOR in the Android > > hardware composer HAL) that can be set by apps like the Android Gears > > test app. > > > > In order to expose this capability to userspace, this series will: > > > > [...] > > > Applied to drm-misc-next, thanks! Where are the IGT and userspace for this uAPI addition?
[PATCH RESEND v2 2/3] drm/bridge: migrate bridge_chains to per-encoder file
Instead of having a single file with all bridge chains, list bridges
under a corresponding per-encoder debugfs directory.
While we are at it, also slightly improve the formatting of the bridge
data: split a single line entry into multiple lines, include the symbol
name of the bridge funcs and add the textual representation of the
bridge ops.
Example of the listing:
$ cat /sys/kernel/debug/dri/0/encoder-0/bridges
bridge[0]: dsi_mgr_bridge_funcs
type: [0] Unknown
ops: [0]
bridge[1]: lt9611uxc_bridge_funcs
type: [11] HDMI-A
OF: /soc@0/geniqup@9c/i2c@994000/hdmi-bridge@2b:lontium,lt9611uxc
ops: [7] detect edid hpd
Reviewed-by: Neil Armstrong
Reviewed-by: Tomi Valkeinen
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_bridge.c | 44 ---
drivers/gpu/drm/drm_debugfs.c | 40 ---
include/drm/drm_bridge.h | 2 --
3 files changed, 37 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 30d66bee0ec6..cee3188adf3d 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1347,50 +1347,6 @@ struct drm_bridge *of_drm_find_bridge(struct device_node
*np)
EXPORT_SYMBOL(of_drm_find_bridge);
#endif
-#ifdef CONFIG_DEBUG_FS
-static int drm_bridge_chains_info(struct seq_file *m, void *data)
-{
- struct drm_debugfs_entry *entry = m->private;
- struct drm_device *dev = entry->dev;
- struct drm_printer p = drm_seq_file_printer(m);
- struct drm_mode_config *config = >mode_config;
- struct drm_encoder *encoder;
- unsigned int bridge_idx = 0;
-
- list_for_each_entry(encoder, >encoder_list, head) {
- struct drm_bridge *bridge;
-
- drm_printf(, "encoder[%u]\n", encoder->base.id);
-
- drm_for_each_bridge_in_chain(encoder, bridge) {
- drm_printf(, "\tbridge[%u] type: %u, ops: %#x",
- bridge_idx, bridge->type, bridge->ops);
-
-#ifdef CONFIG_OF
- if (bridge->of_node)
- drm_printf(, ", OF: %pOFfc", bridge->of_node);
-#endif
-
- drm_printf(, "\n");
-
- bridge_idx++;
- }
- }
-
- return 0;
-}
-
-static const struct drm_debugfs_info drm_bridge_debugfs_list[] = {
- { "bridge_chains", drm_bridge_chains_info, 0 },
-};
-
-void drm_bridge_debugfs_init(struct drm_device *dev)
-{
- drm_debugfs_add_files(dev, drm_bridge_debugfs_list,
- ARRAY_SIZE(drm_bridge_debugfs_list));
-}
-#endif
-
MODULE_AUTHOR("Ajay Kumar ");
MODULE_DESCRIPTION("DRM bridge infrastructure");
MODULE_LICENSE("GPL and additional rights");
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 00406b4f3235..02e7481758c0 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -314,10 +314,8 @@ void drm_debugfs_dev_register(struct drm_device *dev)
drm_framebuffer_debugfs_init(dev);
drm_client_debugfs_init(dev);
}
- if (drm_drv_uses_atomic_modeset(dev)) {
+ if (drm_drv_uses_atomic_modeset(dev))
drm_atomic_debugfs_init(dev);
- drm_bridge_debugfs_init(dev);
- }
}
int drm_debugfs_register(struct drm_minor *minor, int minor_id,
@@ -589,6 +587,38 @@ void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
crtc->debugfs_entry = NULL;
}
+static int bridges_show(struct seq_file *m, void *data)
+{
+ struct drm_encoder *encoder = m->private;
+ struct drm_printer p = drm_seq_file_printer(m);
+ struct drm_bridge *bridge;
+ unsigned int idx = 0;
+
+ drm_for_each_bridge_in_chain(encoder, bridge) {
+ drm_printf(, "bridge[%d]: %ps\n", idx++, bridge->funcs);
+ drm_printf(, "\ttype: [%d] %s\n",
+ bridge->type,
+ drm_get_connector_type_name(bridge->type));
+#ifdef CONFIG_OF
+ if (bridge->of_node)
+ drm_printf(, "\tOF: %pOFfc\n", bridge->of_node);
+#endif
+ drm_printf(, "\tops: [0x%x]", bridge->ops);
+ if (bridge->ops & DRM_BRIDGE_OP_DETECT)
+ drm_puts(, " detect");
+ if (bridge->ops & DRM_BRIDGE_OP_EDID)
+ drm_puts(, " edid");
+ if (bridge->ops & DRM_BRIDGE_OP_HPD)
+ drm_puts(, " hpd");
+ if (bridge->ops & DRM_BRIDGE_OP_MODES)
+ drm_puts(, " modes");
+ drm_puts(, "\n");
+ }
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(bridges);
+
void drm_debugfs_encoder_add(struct drm_encoder *encoder)
{
struct drm_minor *minor = encoder->dev->primary;
@@ -604,6 +634,10 @@ void drm_debugfs_encoder_add(struct drm_encoder *encoder)
[PATCH RESEND v2 1/3] drm/encoder: register per-encoder debugfs dir
Each of connectors and CRTCs used by the DRM device provides debugfs
directory, which is used by several standard debugfs files and can
further be extended by the driver. Add such generic debugfs directories
for encoder.
Reviewed-by: Neil Armstrong
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_debugfs.c | 25 +
drivers/gpu/drm/drm_encoder.c | 4
drivers/gpu/drm/drm_internal.h | 9 +
include/drm/drm_encoder.h | 16 +++-
4 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index f291fb4b359f..00406b4f3235 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -589,4 +589,29 @@ void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
crtc->debugfs_entry = NULL;
}
+void drm_debugfs_encoder_add(struct drm_encoder *encoder)
+{
+ struct drm_minor *minor = encoder->dev->primary;
+ struct dentry *root;
+ char *name;
+
+ name = kasprintf(GFP_KERNEL, "encoder-%d", encoder->index);
+ if (!name)
+ return;
+
+ root = debugfs_create_dir(name, minor->debugfs_root);
+ kfree(name);
+
+ encoder->debugfs_entry = root;
+
+ if (encoder->funcs->debugfs_init)
+ encoder->funcs->debugfs_init(encoder, root);
+}
+
+void drm_debugfs_encoder_remove(struct drm_encoder *encoder)
+{
+ debugfs_remove_recursive(encoder->debugfs_entry);
+ encoder->debugfs_entry = NULL;
+}
+
#endif /* CONFIG_DEBUG_FS */
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 1143bc7f3252..8f2bc6a28482 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -30,6 +30,7 @@
#include
#include "drm_crtc_internal.h"
+#include "drm_internal.h"
/**
* DOC: overview
@@ -74,6 +75,8 @@ int drm_encoder_register_all(struct drm_device *dev)
int ret = 0;
drm_for_each_encoder(encoder, dev) {
+ drm_debugfs_encoder_add(encoder);
+
if (encoder->funcs && encoder->funcs->late_register)
ret = encoder->funcs->late_register(encoder);
if (ret)
@@ -90,6 +93,7 @@ void drm_encoder_unregister_all(struct drm_device *dev)
drm_for_each_encoder(encoder, dev) {
if (encoder->funcs && encoder->funcs->early_unregister)
encoder->funcs->early_unregister(encoder);
+ drm_debugfs_encoder_remove(encoder);
}
}
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index b12c463bc460..7df17e4b0513 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -194,6 +194,8 @@ void drm_debugfs_connector_remove(struct drm_connector
*connector);
void drm_debugfs_crtc_add(struct drm_crtc *crtc);
void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
+void drm_debugfs_encoder_add(struct drm_encoder *encoder);
+void drm_debugfs_encoder_remove(struct drm_encoder *encoder);
#else
static inline void drm_debugfs_dev_fini(struct drm_device *dev)
{
@@ -231,6 +233,13 @@ static inline void drm_debugfs_crtc_crc_add(struct
drm_crtc *crtc)
{
}
+static inline void drm_debugfs_encoder_add(struct drm_encoder *encoder)
+{
+}
+static inline void drm_debugfs_encoder_remove(struct drm_encoder *encoder)
+{
+}
+
#endif
drm_ioctl_t drm_version;
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 3a09682af685..977a9381c8ba 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -60,7 +60,7 @@ struct drm_encoder_funcs {
* @late_register:
*
* This optional hook can be used to register additional userspace
-* interfaces attached to the encoder like debugfs interfaces.
+* interfaces attached to the encoder.
* It is called late in the driver load sequence from
drm_dev_register().
* Everything added from this callback should be unregistered in
* the early_unregister callback.
@@ -81,6 +81,13 @@ struct drm_encoder_funcs {
* before data structures are torndown.
*/
void (*early_unregister)(struct drm_encoder *encoder);
+
+ /**
+* @debugfs_init:
+*
+* Allows encoders to create encoder-specific debugfs files.
+*/
+ void (*debugfs_init)(struct drm_encoder *encoder, struct dentry *root);
};
/**
@@ -184,6 +191,13 @@ struct drm_encoder {
const struct drm_encoder_funcs *funcs;
const struct drm_encoder_helper_funcs *helper_private;
+
+ /**
+* @debugfs_entry:
+*
+* Debugfs directory for this CRTC.
+*/
+ struct dentry *debugfs_entry;
};
#define obj_to_encoder(x) container_of(x, struct drm_encoder, base)
--
2.39.2
[PATCH RESEND v2 3/3] drm/msm/dpu: move encoder status to standard encoder debugfs dir
Now as we have standard per-encoder debugfs directory, move DPU encoder
status file to that directory.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 45 +++--
1 file changed, 6 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1cf7ff6caff4..498983e62f7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -184,7 +184,6 @@ struct dpu_encoder_virt {
struct drm_crtc *crtc;
struct drm_connector *connector;
- struct dentry *debugfs_root;
struct mutex enc_lock;
DECLARE_BITMAP(frame_busy_mask, MAX_PHYS_ENCODERS_PER_VIRTUAL);
void (*crtc_frame_event_cb)(void *, u32 event);
@@ -2108,7 +2107,8 @@ void dpu_encoder_helper_phys_cleanup(struct
dpu_encoder_phys *phys_enc)
#ifdef CONFIG_DEBUG_FS
static int _dpu_encoder_status_show(struct seq_file *s, void *data)
{
- struct dpu_encoder_virt *dpu_enc = s->private;
+ struct drm_encoder *drm_enc = s->private;
+ struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
int i;
mutex_lock(_enc->enc_lock);
@@ -2130,48 +2130,16 @@ static int _dpu_encoder_status_show(struct seq_file *s,
void *data)
DEFINE_SHOW_ATTRIBUTE(_dpu_encoder_status);
-static int _dpu_encoder_init_debugfs(struct drm_encoder *drm_enc)
+static void dpu_encoder_debugfs_init(struct drm_encoder *drm_enc, struct
dentry *root)
{
- struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
-
- char name[12];
-
- if (!drm_enc->dev) {
- DPU_ERROR("invalid encoder or kms\n");
- return -EINVAL;
- }
-
- snprintf(name, sizeof(name), "encoder%u", drm_enc->base.id);
-
- /* create overall sub-directory for the encoder */
- dpu_enc->debugfs_root = debugfs_create_dir(name,
- drm_enc->dev->primary->debugfs_root);
-
/* don't error check these */
debugfs_create_file("status", 0600,
- dpu_enc->debugfs_root, dpu_enc, &_dpu_encoder_status_fops);
-
- return 0;
+ root, drm_enc, &_dpu_encoder_status_fops);
}
#else
-static int _dpu_encoder_init_debugfs(struct drm_encoder *drm_enc)
-{
- return 0;
-}
+#define dpu_encoder_debugfs_init NULL
#endif
-static int dpu_encoder_late_register(struct drm_encoder *encoder)
-{
- return _dpu_encoder_init_debugfs(encoder);
-}
-
-static void dpu_encoder_early_unregister(struct drm_encoder *encoder)
-{
- struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(encoder);
-
- debugfs_remove_recursive(dpu_enc->debugfs_root);
-}
-
static int dpu_encoder_virt_add_phys_encs(
struct msm_display_info *disp_info,
struct dpu_encoder_virt *dpu_enc,
@@ -2355,8 +2323,7 @@ static const struct drm_encoder_helper_funcs
dpu_encoder_helper_funcs = {
static const struct drm_encoder_funcs dpu_encoder_funcs = {
.destroy = dpu_encoder_destroy,
- .late_register = dpu_encoder_late_register,
- .early_unregister = dpu_encoder_early_unregister,
+ .debugfs_init = dpu_encoder_debugfs_init,
};
struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
--
2.39.2
[PATCH RESEND v2 0/3] drm: introduce per-encoder debugfs directory
Resending, patch 1 needs review from DRM core maintainers, but it got no attention since October. Each of connectors and CRTCs used by the DRM device provides debugfs directory, which is used by several standard debugfs files and can further be extended by the driver. Add such generic debugfs directories for encoder. As a showcase for this dir, migrate `bridge_chains' debugfs file (which contains per-encoder data) and MSM custom encoder status to this new debugfs directory. Changes since v1: - Brought back drm_printer usage to bridges_show (Tomi Valkeinen) - Updated the drm/bridge commit message to reflect format changes (Tomi Valkeinen) Dmitry Baryshkov (3): drm/encoder: register per-encoder debugfs dir drm/bridge: migrate bridge_chains to per-encoder file drm/msm/dpu: move encoder status to standard encoder debugfs dir drivers/gpu/drm/drm_bridge.c| 44 -- drivers/gpu/drm/drm_debugfs.c | 65 - drivers/gpu/drm/drm_encoder.c | 4 ++ drivers/gpu/drm/drm_internal.h | 9 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 45 ++ include/drm/drm_bridge.h| 2 - include/drm/drm_encoder.h | 16 - 7 files changed, 96 insertions(+), 89 deletions(-) -- 2.39.2
[PATCH RESEND 5/6] soc: qcom: pmic-glink: switch to DRM_AUX_HPD_BRIDGE
Use the freshly defined DRM_AUX_HPD_BRIDGE instead of open-coding the
same functionality for the DRM bridge chain termination.
Reviewed-by: Bjorn Andersson
Acked-by: Bjorn Andersson
Signed-off-by: Dmitry Baryshkov
---
drivers/soc/qcom/Kconfig | 1 +
drivers/soc/qcom/pmic_glink_altmode.c | 33 ---
2 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index b3634e10f6f5..c954001ae79e 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -86,6 +86,7 @@ config QCOM_PMIC_GLINK
depends on OF
select AUXILIARY_BUS
select QCOM_PDR_HELPERS
+ select DRM_AUX_HPD_BRIDGE
help
The Qualcomm PMIC GLINK driver provides access, over GLINK, to the
USB and battery firmware running on one of the coprocessors in
diff --git a/drivers/soc/qcom/pmic_glink_altmode.c
b/drivers/soc/qcom/pmic_glink_altmode.c
index b78279e2f54c..053b7393e26a 100644
--- a/drivers/soc/qcom/pmic_glink_altmode.c
+++ b/drivers/soc/qcom/pmic_glink_altmode.c
@@ -11,7 +11,7 @@
#include
#include
#include
-#include
+#include
#include
#include
@@ -76,7 +76,7 @@ struct pmic_glink_altmode_port {
struct work_struct work;
- struct drm_bridge bridge;
+ struct device *bridge;
enum typec_orientation orientation;
u16 svid;
@@ -230,10 +230,10 @@ static void pmic_glink_altmode_worker(struct work_struct
*work)
else
pmic_glink_altmode_enable_usb(altmode, alt_port);
- if (alt_port->hpd_state)
- drm_bridge_hpd_notify(_port->bridge,
connector_status_connected);
- else
- drm_bridge_hpd_notify(_port->bridge,
connector_status_disconnected);
+ drm_aux_hpd_bridge_notify(alt_port->bridge,
+ alt_port->hpd_state ?
+ connector_status_connected :
+ connector_status_disconnected);
pmic_glink_altmode_request(altmode, ALTMODE_PAN_ACK, alt_port->index);
};
@@ -365,16 +365,6 @@ static void pmic_glink_altmode_callback(const void *data,
size_t len, void *priv
}
}
-static int pmic_glink_altmode_attach(struct drm_bridge *bridge,
-enum drm_bridge_attach_flags flags)
-{
- return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
-}
-
-static const struct drm_bridge_funcs pmic_glink_altmode_bridge_funcs = {
- .attach = pmic_glink_altmode_attach,
-};
-
static void pmic_glink_altmode_put_retimer(void *data)
{
typec_retimer_put(data);
@@ -464,15 +454,10 @@ static int pmic_glink_altmode_probe(struct
auxiliary_device *adev,
alt_port->index = port;
INIT_WORK(_port->work, pmic_glink_altmode_worker);
- alt_port->bridge.funcs = _glink_altmode_bridge_funcs;
- alt_port->bridge.of_node = to_of_node(fwnode);
- alt_port->bridge.ops = DRM_BRIDGE_OP_HPD;
- alt_port->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
-
- ret = devm_drm_bridge_add(dev, _port->bridge);
- if (ret) {
+ alt_port->bridge = drm_dp_hpd_bridge_register(dev,
to_of_node(fwnode));
+ if (IS_ERR(alt_port->bridge)) {
fwnode_handle_put(fwnode);
- return ret;
+ return PTR_ERR(alt_port->bridge);
}
alt_port->dp_alt.svid = USB_TYPEC_DP_SID;
--
2.39.2
[PATCH RESEND 6/6] usb: typec: qcom-pmic-typec: switch to DRM_AUX_HPD_BRIDGE
Use the freshly defined DRM_AUX_HPD_BRIDGE instead of open-coding the
same functionality for the DRM bridge chain termination.
Acked-by: Bryan O'Donoghue
Signed-off-by: Dmitry Baryshkov
---
drivers/usb/typec/tcpm/Kconfig| 1 +
drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 41 +++
2 files changed, 7 insertions(+), 35 deletions(-)
diff --git a/drivers/usb/typec/tcpm/Kconfig b/drivers/usb/typec/tcpm/Kconfig
index 0b2993fef564..64d5421c69e6 100644
--- a/drivers/usb/typec/tcpm/Kconfig
+++ b/drivers/usb/typec/tcpm/Kconfig
@@ -80,6 +80,7 @@ config TYPEC_QCOM_PMIC
tristate "Qualcomm PMIC USB Type-C Port Controller Manager driver"
depends on ARCH_QCOM || COMPILE_TEST
depends on DRM || DRM=n
+ select DRM_AUX_HPD_BRIDGE if DRM_BRIDGE
help
A Type-C port and Power Delivery driver which aggregates two
discrete pieces of silicon in the PM8150b PMIC block: the
diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
index 581199d37b49..1a2b4bddaa97 100644
--- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
+++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
@@ -18,7 +18,7 @@
#include
#include
-#include
+#include
#include "qcom_pmic_typec_pdphy.h"
#include "qcom_pmic_typec_port.h"
@@ -36,7 +36,6 @@ struct pmic_typec {
struct pmic_typec_port *pmic_typec_port;
boolvbus_enabled;
struct mutexlock; /* VBUS state serialization */
- struct drm_bridge bridge;
};
#define tcpc_to_tcpm(_tcpc_) container_of(_tcpc_, struct pmic_typec, tcpc)
@@ -150,35 +149,6 @@ static int qcom_pmic_typec_init(struct tcpc_dev *tcpc)
return 0;
}
-#if IS_ENABLED(CONFIG_DRM)
-static int qcom_pmic_typec_attach(struct drm_bridge *bridge,
-enum drm_bridge_attach_flags flags)
-{
- return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
-}
-
-static const struct drm_bridge_funcs qcom_pmic_typec_bridge_funcs = {
- .attach = qcom_pmic_typec_attach,
-};
-
-static int qcom_pmic_typec_init_drm(struct pmic_typec *tcpm)
-{
- tcpm->bridge.funcs = _pmic_typec_bridge_funcs;
-#ifdef CONFIG_OF
- tcpm->bridge.of_node = of_get_child_by_name(tcpm->dev->of_node,
"connector");
-#endif
- tcpm->bridge.ops = DRM_BRIDGE_OP_HPD;
- tcpm->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
-
- return devm_drm_bridge_add(tcpm->dev, >bridge);
-}
-#else
-static int qcom_pmic_typec_init_drm(struct pmic_typec *tcpm)
-{
- return 0;
-}
-#endif
-
static int qcom_pmic_typec_probe(struct platform_device *pdev)
{
struct pmic_typec *tcpm;
@@ -186,6 +156,7 @@ static int qcom_pmic_typec_probe(struct platform_device
*pdev)
struct device_node *np = dev->of_node;
const struct pmic_typec_resources *res;
struct regmap *regmap;
+ struct device *bridge_dev;
u32 base[2];
int ret;
@@ -241,14 +212,14 @@ static int qcom_pmic_typec_probe(struct platform_device
*pdev)
mutex_init(>lock);
platform_set_drvdata(pdev, tcpm);
- ret = qcom_pmic_typec_init_drm(tcpm);
- if (ret)
- return ret;
-
tcpm->tcpc.fwnode = device_get_named_child_node(tcpm->dev, "connector");
if (!tcpm->tcpc.fwnode)
return -EINVAL;
+ bridge_dev = drm_dp_hpd_bridge_register(tcpm->dev,
to_of_node(tcpm->tcpc.fwnode));
+ if (IS_ERR(bridge_dev))
+ return PTR_ERR(bridge_dev);
+
tcpm->tcpm_port = tcpm_register_port(tcpm->dev, >tcpc);
if (IS_ERR(tcpm->tcpm_port)) {
ret = PTR_ERR(tcpm->tcpm_port);
--
2.39.2
[PATCH RESEND 4/6] drm/bridge: implement generic DP HPD bridge
Several USB-C controllers implement a pretty simple DRM bridge which
implements just the HPD notification operations. Add special helper
for creating such simple bridges.
Acked-by: Neil Armstrong
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/bridge/Kconfig | 8 ++
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/aux-hpd-bridge.c | 164
include/drm/bridge/aux-bridge.h | 18 +++
4 files changed, 191 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/aux-hpd-bridge.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index f12eab62799f..19d2dc05c397 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -21,6 +21,14 @@ config DRM_AUX_BRIDGE
Simple transparent bridge that is used by several non-DRM drivers to
build bridges chain.
+config DRM_AUX_HPD_BRIDGE
+ tristate
+ depends on DRM_BRIDGE && OF
+ select AUXILIARY_BUS
+ help
+ Simple bridge that terminates the bridge chain and provides HPD
+ support.
+
menu "Display Interface Bridges"
depends on DRM && DRM_BRIDGE
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 918e3bfff079..017b5832733b 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o
+obj-$(CONFIG_DRM_AUX_HPD_BRIDGE) += aux-hpd-bridge.o
obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o
obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o
diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
new file mode 100644
index ..4defac8ec63f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Author: Dmitry Baryshkov
+ */
+#include
+#include
+#include
+
+#include
+#include
+
+static DEFINE_IDA(drm_aux_hpd_bridge_ida);
+
+struct drm_aux_hpd_bridge_data {
+ struct drm_bridge bridge;
+ struct device *dev;
+};
+
+static void drm_aux_hpd_bridge_release(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+ ida_free(_aux_hpd_bridge_ida, adev->id);
+
+ of_node_put(adev->dev.platform_data);
+
+ kfree(adev);
+}
+
+static void drm_aux_hpd_bridge_unregister_adev(void *_adev)
+{
+ struct auxiliary_device *adev = _adev;
+
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+}
+
+/**
+ * drm_dp_hpd_bridge_register - Create a simple HPD DisplayPort bridge
+ * @parent: device instance providing this bridge
+ * @np: device node pointer corresponding to this bridge instance
+ *
+ * Creates a simple DRM bridge with the type set to
+ * DRM_MODE_CONNECTOR_DisplayPort, which terminates the bridge chain and is
+ * able to send the HPD events.
+ *
+ * Return: device instance that will handle created bridge or an error code
+ * encoded into the pointer.
+ */
+struct device *drm_dp_hpd_bridge_register(struct device *parent,
+ struct device_node *np)
+{
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+ if (!adev)
+ return ERR_PTR(-ENOMEM);
+
+ ret = ida_alloc(_aux_hpd_bridge_ida, GFP_KERNEL);
+ if (ret < 0) {
+ kfree(adev);
+ return ERR_PTR(ret);
+ }
+
+ adev->id = ret;
+ adev->name = "dp_hpd_bridge";
+ adev->dev.parent = parent;
+ adev->dev.of_node = parent->of_node;
+ adev->dev.release = drm_aux_hpd_bridge_release;
+ adev->dev.platform_data = np;
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ ida_free(_aux_hpd_bridge_ida, adev->id);
+ kfree(adev);
+ return ERR_PTR(ret);
+ }
+
+ ret = auxiliary_device_add(adev);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ return ERR_PTR(ret);
+ }
+
+ ret = devm_add_action_or_reset(parent,
drm_aux_hpd_bridge_unregister_adev, adev);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return >dev;
+
+}
+EXPORT_SYMBOL_GPL(drm_dp_hpd_bridge_register);
+
+/**
+ * drm_aux_hpd_bridge_notify - notify hot plug detection events
+ * @dev: device created for the HPD bridge
+ * @status: output connection status
+ *
+ * A wrapper around drm_bridge_hpd_notify() that is used to report hot plug
+ * detection events for bridges created via drm_dp_hpd_bridge_register().
+ *
+ * This function shall be called in a context that can sleep.
+ */
+void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status
status)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
[PATCH RESEND 2/6] phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE
Switch to using the new DRM_AUX_BRIDGE helper to create the
transparent DRM bridge device instead of handcoding corresponding
functionality.
Acked-by: Vinod Koul
Signed-off-by: Dmitry Baryshkov
---
drivers/phy/qualcomm/Kconfig | 2 +-
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 44 ++-
2 files changed, 3 insertions(+), 43 deletions(-)
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index d891058b7c39..846f8c99547f 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -63,7 +63,7 @@ config PHY_QCOM_QMP_COMBO
depends on DRM || DRM=n
select GENERIC_PHY
select MFD_SYSCON
- select DRM_PANEL_BRIDGE if DRM
+ select DRM_AUX_BRIDGE if DRM_BRIDGE
help
Enable this to support the QMP Combo PHY transceiver that is used
with USB3 and DisplayPort controllers on Qualcomm chips.
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 0417856b8e7b..435cd849e82e 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -21,7 +21,7 @@
#include
#include
-#include
+#include
#include
@@ -1419,8 +1419,6 @@ struct qmp_combo {
struct clk_hw dp_link_hw;
struct clk_hw dp_pixel_hw;
- struct drm_bridge bridge;
-
struct typec_switch_dev *sw;
enum typec_orientation orientation;
};
@@ -3191,44 +3189,6 @@ static int qmp_combo_typec_switch_register(struct
qmp_combo *qmp)
}
#endif
-#if IS_ENABLED(CONFIG_DRM)
-static int qmp_combo_bridge_attach(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags)
-{
- struct qmp_combo *qmp = container_of(bridge, struct qmp_combo, bridge);
- struct drm_bridge *next_bridge;
-
- if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
- return -EINVAL;
-
- next_bridge = devm_drm_of_get_bridge(qmp->dev, qmp->dev->of_node, 0, 0);
- if (IS_ERR(next_bridge)) {
- dev_err(qmp->dev, "failed to acquire drm_bridge: %pe\n",
next_bridge);
- return PTR_ERR(next_bridge);
- }
-
- return drm_bridge_attach(bridge->encoder, next_bridge, bridge,
-DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-}
-
-static const struct drm_bridge_funcs qmp_combo_bridge_funcs = {
- .attach = qmp_combo_bridge_attach,
-};
-
-static int qmp_combo_dp_register_bridge(struct qmp_combo *qmp)
-{
- qmp->bridge.funcs = _combo_bridge_funcs;
- qmp->bridge.of_node = qmp->dev->of_node;
-
- return devm_drm_bridge_add(qmp->dev, >bridge);
-}
-#else
-static int qmp_combo_dp_register_bridge(struct qmp_combo *qmp)
-{
- return 0;
-}
-#endif
-
static int qmp_combo_parse_dt_lecacy_dp(struct qmp_combo *qmp, struct
device_node *np)
{
struct device *dev = qmp->dev;
@@ -3440,7 +3400,7 @@ static int qmp_combo_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = qmp_combo_dp_register_bridge(qmp);
+ ret = drm_aux_bridge_register(dev);
if (ret)
return ret;
--
2.39.2
[PATCH RESEND 3/6] usb: typec: nb7vpq904m: switch to DRM_AUX_BRIDGE
Switch to using the new DRM_AUX_BRIDGE helper to create the
transparent DRM bridge device instead of handcoding corresponding
functionality.
Reviewed-by: Heikki Krogerus
Acked-by: Greg Kroah-Hartman
Signed-off-by: Dmitry Baryshkov
---
drivers/usb/typec/mux/Kconfig | 2 +-
drivers/usb/typec/mux/nb7vpq904m.c | 44 ++
2 files changed, 3 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
index 816b9bd08355..5120942f309d 100644
--- a/drivers/usb/typec/mux/Kconfig
+++ b/drivers/usb/typec/mux/Kconfig
@@ -40,7 +40,7 @@ config TYPEC_MUX_NB7VPQ904M
tristate "On Semiconductor NB7VPQ904M Type-C redriver driver"
depends on I2C
depends on DRM || DRM=n
- select DRM_PANEL_BRIDGE if DRM
+ select DRM_AUX_BRIDGE if DRM_BRIDGE
select REGMAP_I2C
help
Say Y or M if your system has a On Semiconductor NB7VPQ904M Type-C
diff --git a/drivers/usb/typec/mux/nb7vpq904m.c
b/drivers/usb/typec/mux/nb7vpq904m.c
index cda206cf0c38..b17826713753 100644
--- a/drivers/usb/typec/mux/nb7vpq904m.c
+++ b/drivers/usb/typec/mux/nb7vpq904m.c
@@ -11,7 +11,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -70,8 +70,6 @@ struct nb7vpq904m {
bool swap_data_lanes;
struct typec_switch *typec_switch;
- struct drm_bridge bridge;
-
struct mutex lock; /* protect non-concurrent retimer & switch */
enum typec_orientation orientation;
@@ -297,44 +295,6 @@ static int nb7vpq904m_retimer_set(struct typec_retimer
*retimer, struct typec_re
return ret;
}
-#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DRM_PANEL_BRIDGE)
-static int nb7vpq904m_bridge_attach(struct drm_bridge *bridge,
- enum drm_bridge_attach_flags flags)
-{
- struct nb7vpq904m *nb7 = container_of(bridge, struct nb7vpq904m,
bridge);
- struct drm_bridge *next_bridge;
-
- if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
- return -EINVAL;
-
- next_bridge = devm_drm_of_get_bridge(>client->dev,
nb7->client->dev.of_node, 0, 0);
- if (IS_ERR(next_bridge)) {
- dev_err(>client->dev, "failed to acquire drm_bridge:
%pe\n", next_bridge);
- return PTR_ERR(next_bridge);
- }
-
- return drm_bridge_attach(bridge->encoder, next_bridge, bridge,
-DRM_BRIDGE_ATTACH_NO_CONNECTOR);
-}
-
-static const struct drm_bridge_funcs nb7vpq904m_bridge_funcs = {
- .attach = nb7vpq904m_bridge_attach,
-};
-
-static int nb7vpq904m_register_bridge(struct nb7vpq904m *nb7)
-{
- nb7->bridge.funcs = _bridge_funcs;
- nb7->bridge.of_node = nb7->client->dev.of_node;
-
- return devm_drm_bridge_add(>client->dev, >bridge);
-}
-#else
-static int nb7vpq904m_register_bridge(struct nb7vpq904m *nb7)
-{
- return 0;
-}
-#endif
-
static const struct regmap_config nb7_regmap = {
.max_register = 0x1f,
.reg_bits = 8,
@@ -461,7 +421,7 @@ static int nb7vpq904m_probe(struct i2c_client *client)
gpiod_set_value(nb7->enable_gpio, 1);
- ret = nb7vpq904m_register_bridge(nb7);
+ ret = drm_aux_bridge_register(dev);
if (ret)
goto err_disable_gpio;
--
2.39.2
[PATCH RESEND 0/6] drm: simplify support for transparent DRM bridges
Greg, could you please ack the last patch to be merged through the drm-misc tree? You have acked patch 3, but since that time I've added patches 4-6. Supporting DP/USB-C can result in a chain of several transparent bridges (PHY, redrivers, mux, etc). All attempts to implement DP support in a different way resulted either in series of hacks or in device tree not reflecting the actual hardware design. This results in drivers having similar boilerplate code for such bridges. Next, these drivers are susceptible to -EPROBE_DEFER loops: the next bridge can either be probed from the bridge->attach callback, when it is too late to return -EPROBE_DEFER, or from the probe() callback, when the next bridge might not yet be available, because it depends on the resources provided by the probing device. Device links can not fully solve this problem since there are mutual dependencies between adjancent devices. Last, but not least, this results in the the internal knowledge of DRM subsystem slowly diffusing into other subsystems, like PHY or USB/TYPEC. To solve all these issues, define a separate DRM helper, which creates separate aux device just for the bridge. During probe such aux device doesn't result in the EPROBE_DEFER loops. Instead it allows the device drivers to probe properly, according to the actual resource dependencies. The bridge auxdevs are then probed when the next bridge becomes available, sparing drivers from drm_bridge_attach() returning -EPROBE_DEFER. Changes since v5: - Removed extra semicolon in !DRM_AUX_HPD_BRIDGE stubs definition. Changes since v4: - Added documentation for new API (Sima) - Added generic code to handle "last mile" DP bridges implementing just the HPD functionality. - Rebased on top of linux-next to be able to drop #ifdef's around drm_bridge->of_node Changes since v3: - Moved bridge driver to gpu/drm/bridge (Neil Armstrong) - Renamed it to aux-bridge (since there is already a simple_bridge driver) - Made CONFIG_OF mandatory for this driver (Neil Armstrong) - Added missing kfree and ida_free (Dan Carpenter) Changes since v2: - ifdef'ed bridge->of_node access (LKP) Changes since v1: - Added EXPORT_SYMBOL_GPL / MODULE_LICENSE / etc. to drm_simple_bridge Dmitry Baryshkov (6): drm/bridge: add transparent bridge helper phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE usb: typec: nb7vpq904m: switch to DRM_AUX_BRIDGE drm/bridge: implement generic DP HPD bridge soc: qcom: pmic-glink: switch to DRM_AUX_HPD_BRIDGE usb: typec: qcom-pmic-typec: switch to DRM_AUX_HPD_BRIDGE drivers/gpu/drm/bridge/Kconfig| 17 ++ drivers/gpu/drm/bridge/Makefile | 2 + drivers/gpu/drm/bridge/aux-bridge.c | 140 +++ drivers/gpu/drm/bridge/aux-hpd-bridge.c | 164 ++ drivers/phy/qualcomm/Kconfig | 2 +- drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 44 + drivers/soc/qcom/Kconfig | 1 + drivers/soc/qcom/pmic_glink_altmode.c | 33 +--- drivers/usb/typec/mux/Kconfig | 2 +- drivers/usb/typec/mux/nb7vpq904m.c| 44 + drivers/usb/typec/tcpm/Kconfig| 1 + drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c | 41 + include/drm/bridge/aux-bridge.h | 37 13 files changed, 383 insertions(+), 145 deletions(-) create mode 100644 drivers/gpu/drm/bridge/aux-bridge.c create mode 100644 drivers/gpu/drm/bridge/aux-hpd-bridge.c create mode 100644 include/drm/bridge/aux-bridge.h -- 2.39.2
[PATCH RESEND 1/6] drm/bridge: add transparent bridge helper
Define a helper for creating simple transparent bridges which serve the
only purpose of linking devices into the bridge chain up to the last
bridge representing the connector. This is especially useful for
DP/USB-C bridge chains, which can span across several devices, but do
not require any additional functionality from the intermediate bridges.
Reviewed-by: Neil Armstrong
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/bridge/Kconfig | 9 ++
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/aux-bridge.c | 140
include/drm/bridge/aux-bridge.h | 19
4 files changed, 169 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/aux-bridge.c
create mode 100644 include/drm/bridge/aux-bridge.h
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index ba82a1142adf..f12eab62799f 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -12,6 +12,15 @@ config DRM_PANEL_BRIDGE
help
DRM bridge wrapper of DRM panels
+config DRM_AUX_BRIDGE
+ tristate
+ depends on DRM_BRIDGE && OF
+ select AUXILIARY_BUS
+ select DRM_PANEL_BRIDGE
+ help
+ Simple transparent bridge that is used by several non-DRM drivers to
+ build bridges chain.
+
menu "Display Interface Bridges"
depends on DRM && DRM_BRIDGE
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 2b892b7ed59e..918e3bfff079 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o
obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o
obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o
diff --git a/drivers/gpu/drm/bridge/aux-bridge.c
b/drivers/gpu/drm/bridge/aux-bridge.c
new file mode 100644
index ..6245976b8fef
--- /dev/null
+++ b/drivers/gpu/drm/bridge/aux-bridge.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Author: Dmitry Baryshkov
+ */
+#include
+#include
+
+#include
+#include
+
+static DEFINE_IDA(drm_aux_bridge_ida);
+
+static void drm_aux_bridge_release(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+ ida_free(_aux_bridge_ida, adev->id);
+
+ kfree(adev);
+}
+
+static void drm_aux_bridge_unregister_adev(void *_adev)
+{
+ struct auxiliary_device *adev = _adev;
+
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+}
+
+/**
+ * drm_aux_bridge_register - Create a simple bridge device to link the chain
+ * @parent: device instance providing this bridge
+ *
+ * Creates a simple DRM bridge that doesn't implement any drm_bridge
+ * operations. Such bridges merely fill a place in the bridge chain linking
+ * surrounding DRM bridges.
+ *
+ * Return: zero on success, negative error code on failure
+ */
+int drm_aux_bridge_register(struct device *parent)
+{
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+ if (!adev)
+ return -ENOMEM;
+
+ ret = ida_alloc(_aux_bridge_ida, GFP_KERNEL);
+ if (ret < 0) {
+ kfree(adev);
+ return ret;
+ }
+
+ adev->id = ret;
+ adev->name = "aux_bridge";
+ adev->dev.parent = parent;
+ adev->dev.of_node = parent->of_node;
+ adev->dev.release = drm_aux_bridge_release;
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ ida_free(_aux_bridge_ida, adev->id);
+ kfree(adev);
+ return ret;
+ }
+
+ ret = auxiliary_device_add(adev);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ return ret;
+ }
+
+ return devm_add_action_or_reset(parent, drm_aux_bridge_unregister_adev,
adev);
+}
+EXPORT_SYMBOL_GPL(drm_aux_bridge_register);
+
+struct drm_aux_bridge_data {
+ struct drm_bridge bridge;
+ struct drm_bridge *next_bridge;
+ struct device *dev;
+};
+
+static int drm_aux_bridge_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct drm_aux_bridge_data *data;
+
+ if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
+ return -EINVAL;
+
+ data = container_of(bridge, struct drm_aux_bridge_data, bridge);
+
+ return drm_bridge_attach(bridge->encoder, data->next_bridge, bridge,
+DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+}
+
+static const struct drm_bridge_funcs drm_aux_bridge_funcs = {
+ .attach = drm_aux_bridge_attach,
+};
+
+static int drm_aux_bridge_probe(struct auxiliary_device *auxdev,
+ const struct auxiliary_device_id *id)
+{
+ struct drm_aux_bridge_data
Re: [PATCH v2 4/7] drm/msm/gem: Split out submit_unpin_objects() helper
On 21/11/2023 02:38, Rob Clark wrote: From: Rob Clark Untangle unpinning from unlock/unref loop. The unpin only happens in error paths so it is easier to decouple from the normal unlock path. Since we never have an intermediate state where a subset of buffers are pinned (ie. we never bail out of the pin or unpin loops) we can replace the bo state flag bit with a global flag in the submit. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.h| 6 +++--- drivers/gpu/drm/msm/msm_gem_submit.c | 22 +- drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++- 3 files changed, 22 insertions(+), 9 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v2 5/7] drm/msm/gem: Cleanup submit_cleanup_bo()
On 21/11/2023 02:38, Rob Clark wrote:
From: Rob Clark
Now that it only handles unlock duty, drop the superfluous arg and
rename it.
Signed-off-by: Rob Clark
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/msm_gem_submit.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index d001bf286606..603f04d851d9 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -248,14 +248,10 @@ static int submit_lookup_cmds(struct msm_gem_submit
*submit,
return ret;
}
-/* Unwind bo state, according to cleanup_flags. In the success case, only
- * the lock is dropped at the end of the submit (and active/pin ref is dropped
- * later when the submit is retired).
- */
-static void submit_cleanup_bo(struct msm_gem_submit *submit, int i,
- unsigned cleanup_flags)
+static void submit_unlock_bo(struct msm_gem_submit *submit, int i)
{
struct drm_gem_object *obj = submit->bos[i].obj;
+ unsigned cleanup_flags = BO_LOCKED;
Nit: checkpatch will warn here, it should be unsigned int.
unsigned flags = submit->bos[i].flags & cleanup_flags;
/*
@@ -304,10 +300,10 @@ static int submit_lock_objects(struct msm_gem_submit
*submit)
}
for (; i >= 0; i--)
- submit_cleanup_bo(submit, i, BO_LOCKED);
+ submit_unlock_bo(submit, i);
if (slow_locked > 0)
- submit_cleanup_bo(submit, slow_locked, BO_LOCKED);
+ submit_unlock_bo(submit, slow_locked);
if (ret == -EDEADLK) {
struct drm_gem_object *obj = submit->bos[contended].obj;
@@ -533,7 +529,6 @@ static int submit_reloc(struct msm_gem_submit *submit,
struct drm_gem_object *ob
*/
static void submit_cleanup(struct msm_gem_submit *submit, bool error)
{
- unsigned cleanup_flags = BO_LOCKED;
unsigned i;
if (error)
@@ -541,7 +536,7 @@ static void submit_cleanup(struct msm_gem_submit *submit,
bool error)
for (i = 0; i < submit->nr_bos; i++) {
struct drm_gem_object *obj = submit->bos[i].obj;
- submit_cleanup_bo(submit, i, cleanup_flags);
+ submit_unlock_bo(submit, i);
if (error)
drm_gem_object_put(obj);
}
--
With best wishes
Dmitry
Re: [PATCH v2 1/7] drm/msm/gem: Remove "valid" tracking
On 21/11/2023 02:38, Rob Clark wrote: From: Rob Clark This was a small optimization for pre-soft-pin userspace. But mesa switched to soft-pin nearly 5yrs ago. So lets drop the optimization and simplify the code. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.h| 2 -- drivers/gpu/drm/msm/msm_gem_submit.c | 44 +--- 2 files changed, 8 insertions(+), 38 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH] drm/msm/dpu: Correct UBWC settings for sc8280xp
On Thu, 30 Nov 2023 11:21:18 -0800, Rob Clark wrote: > The UBWC settings need to match between the display and GPU. When we > updated the GPU settings, we forgot to make the corresponding update on > the display side. > > Applied, thanks! [1/1] drm/msm/dpu: Correct UBWC settings for sc8280xp https://gitlab.freedesktop.org/lumag/msm/-/commit/0b414c731432 Best regards, -- Dmitry Baryshkov
Re: [PATCH v9 0/7] incorporate pm runtime framework and eDP clean up
On Fri, 01 Dec 2023 15:19:42 -0800, Kuogee Hsieh wrote: > The purpose of this patch series is to incorporate pm runtime framework > into MSM eDP/DP driver so that eDP panel can be detected by DRM eDP panel > driver during system probe time. During incorporating procedure, original > customized pm realted fucntions, such as dp_pm_prepare(), dp_pm_suspend(), > dp_pm_resume() and dp_pm_prepare(), are removed and replaced with functions > provided by pm runtiem framework such as pm_runtime_force_suspend() and > pm_runtime_force_resume(). In addition, both eDP aux-bus and irq handler > are bound at system probe time too. > > [...] Applied, thanks! [1/7] drm/msm/dp: tie dp_display_irq_handler() with dp driver https://gitlab.freedesktop.org/lumag/msm/-/commit/82c2a5751227 [2/7] drm/msm/dp: rename is_connected with link_ready https://gitlab.freedesktop.org/lumag/msm/-/commit/aa1131204e58 [3/7] drm/msm/dp: use drm_bridge_hpd_notify() to report HPD status changes https://gitlab.freedesktop.org/lumag/msm/-/commit/e467e0bde881 [4/7] drm/msm/dp: move parser->parse() and dp_power_client_init() to probe https://gitlab.freedesktop.org/lumag/msm/-/commit/9179fd9596a4 [5/7] drm/msm/dp: incorporate pm_runtime framework into DP driver https://gitlab.freedesktop.org/lumag/msm/-/commit/5814b8bf086a [6/7] drm/msm/dp: delete EV_HPD_INIT_SETUP https://gitlab.freedesktop.org/lumag/msm/-/commit/2b3aabc9caa2 [7/7] drm/msm/dp: move of_dp_aux_populate_bus() to eDP probe() https://gitlab.freedesktop.org/lumag/msm/-/commit/e2969ee30252 Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm/dpu: enable smartdma on sm8350
On Fri, 08 Sep 2023 12:33:13 -0700, Abhinav Kumar wrote: > To support high resolutions on sm8350, enable smartdma > in its catalog. > > Applied, thanks! [1/1] drm/msm/dpu: enable smartdma on sm8350 https://gitlab.freedesktop.org/lumag/msm/-/commit/921e32bf6c0c Best regards, -- Dmitry Baryshkov
Re: [PATCH v2 1/2] drm/msm/dpu: fail dpu_plane_atomic_check() based on mdp clk limits
On Mon, 11 Sep 2023 15:16:26 -0700, Abhinav Kumar wrote: > Currently, dpu_plane_atomic_check() does not check whether the > plane can process the image without exceeding the per chipset > limits for MDP clock. This leads to underflow issues because the > SSPP is not able to complete the processing for the data rate of > the display. > > Fail the dpu_plane_atomic_check() if the SSPP cannot process the > image without exceeding the MDP clock limits. > > [...] Applied, thanks! [2/2] drm/msm/dpu: try multirect based on mdp clock limits https://gitlab.freedesktop.org/lumag/msm/-/commit/e6c0de5f4450 Best regards, -- Dmitry Baryshkov
Re: [PATCH] dt-bindings: display/msm: qcom, sm8250-mdss: add DisplayPort controller node
On Tue, 07 Nov 2023 11:36:00 +0100, Krzysztof Kozlowski wrote:
> Document the DisplayPort controller node in MDSS binding, already used
> in DTS:
>
> sm8250-xiaomi-elish-boe.dtb: display-subsystem@ae0: Unevaluated
> properties are not allowed ('displayport-controller@ae9' was unexpected)
>
>
Applied, thanks!
[1/1] dt-bindings: display/msm: qcom,sm8250-mdss: add DisplayPort controller
node
https://gitlab.freedesktop.org/lumag/msm/-/commit/52e36770b174
Best regards,
--
Dmitry Baryshkov
Re: [PATCH] drm/msm/mdp4: flush vblank event on disable
On Tue, 28 Nov 2023 00:54:01 +0300, Dmitry Baryshkov wrote: > Flush queued events when disabling the crtc. This avoids timeouts when > we come back and wait for dependencies (like the previous frame's > flip_done). > > Applied, thanks! [1/1] drm/msm/mdp4: flush vblank event on disable https://gitlab.freedesktop.org/lumag/msm/-/commit/c6721b3c6423 Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm/dpu: Add missing safe_lut_tbl in sc8180x catalog
On Thu, 30 Nov 2023 16:35:01 -0800, Bjorn Andersson wrote: > Similar to SC8280XP, the misconfigured SAFE logic causes rather > significant delays in __arm_smmu_tlb_sync(), resulting in poor > performance for things such as USB. > > Introduce appropriate SAFE values for SC8180X to correct this. > > > [...] Applied, thanks! [1/1] drm/msm/dpu: Add missing safe_lut_tbl in sc8180x catalog https://gitlab.freedesktop.org/lumag/msm/-/commit/7cc2621f16b6 Best regards, -- Dmitry Baryshkov
Re: [PATCH v3] drm/msm/dpu: Capture dpu snapshot when frame_done_timer timeouts
On Thu, 30 Nov 2023 14:47:37 -0800, Paloma Arellano wrote: > Trigger a devcoredump to dump dpu registers and capture the drm atomic > state when the frame_done_timer timeouts. > > v2: Optimize the format in which frame_done_timeout_cnt is incremented > v3: Describe parameter frame_done_timeout_cnt in dpu_encoder_virt > > > [...] Applied, thanks! [1/1] drm/msm/dpu: Capture dpu snapshot when frame_done_timer timeouts https://gitlab.freedesktop.org/lumag/msm/-/commit/9cad81143ef0 Best regards, -- Dmitry Baryshkov
Re: [PATCH v2 0/2] DSIPHY RPM
On Tue, 20 Jun 2023 13:43:19 +0200, Konrad Dybcio wrote: > Some recent SoCs use power rails that we model as GENPDs to power the > DSIPHY. This series attempts to make such configurations suspendable. > > Tested on SM6375. > > Applied, thanks! [1/2] drm/msm/dsi: Use pm_runtime_resume_and_get to prevent refcnt leaks https://gitlab.freedesktop.org/lumag/msm/-/commit/3d07a411b4fa [2/2] drm/msm/dsi: Enable runtime PM https://gitlab.freedesktop.org/lumag/msm/-/commit/6ab502bc1cf3 Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm/dpu: enable SmartDMA on SM8450
On Mon, 09 Oct 2023 19:56:27 +0300, Dmitry Baryshkov wrote: > Enable the SmartDMA / multirect support on the SM8450 platform to > support higher resoltion modes. > > Applied, thanks! [1/1] drm/msm/dpu: enable SmartDMA on SM8450 https://gitlab.freedesktop.org/lumag/msm/-/commit/a9bd555de5e9 Best regards, -- Dmitry Baryshkov
Re: [PATCH v3 00/12] RB1/QCM2290 features
On Wed, 29 Nov 2023 15:43:57 +0100, Konrad Dybcio wrote: > This series brings: > - interconnect plumbing > - display setup > > for QCM2290/QRB2210 and > > - CAN bus controller > - HDMI display > - wifi fw variant name > > [...] Applied, thanks! [01/12] dt-bindings: display: msm: qcm2290-mdss: Use the non-deprecated DSI compat https://gitlab.freedesktop.org/lumag/msm/-/commit/25daacc60394 [02/12] dt-bindings: display: msm: Add reg bus and rotator interconnects https://gitlab.freedesktop.org/lumag/msm/-/commit/a1ed5860efd3 Best regards, -- Dmitry Baryshkov
Re: [PATCH] dt-bindings: display/msm: qcom, sm8150-mdss: correct DSI PHY compatible
On Sat, 11 Nov 2023 15:20:17 +0100, Krzysztof Kozlowski wrote: > Qualcomm SM8150 MDSS comes with a bit different 7nm DSI PHY with its own > compatible. DTS already use it: > > sa8155p-adp.dtb: display-subsystem@ae0: phy@ae94400:compatible:0: > 'qcom,dsi-phy-7nm' was expected > > Applied, thanks! [1/1] dt-bindings: display/msm: qcom,sm8150-mdss: correct DSI PHY compatible https://gitlab.freedesktop.org/lumag/msm/-/commit/1cd83dfe9a58 Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm/dp: cleanup debugfs handling
On Thu, 19 Oct 2023 13:44:19 +0300, Dmitry Baryshkov wrote: > Currently there are two subdirs for DP debugfs files, e.g. DP-1, created > by the drm core for the connector, and the msm_dp-DP-1, created by the > DP driver itself. Merge those two, so that there are no extraneous > connector-related subdirs. > > Applied, thanks! [1/1] drm/msm/dp: cleanup debugfs handling https://gitlab.freedesktop.org/lumag/msm/-/commit/ab8420418c2e Best regards, -- Dmitry Baryshkov
Re: [PATCH 00/17] drm/msm/mdp[45]: use managed memory allocations
On Sat, 08 Jul 2023 04:03:50 +0300, Dmitry Baryshkov wrote: > Follow the DPU patchset ([1]) and use devm_ and drmm_ functions to > allocate long-living data structures in mdp4 and mdp5 drivers. > > [1] https://patchwork.freedesktop.org/series/120366/ > > Dmitry Baryshkov (17): > drm/msm: add arrays listing formats supported by MDP4/MDP5 hardware > drm/msm/mdp5: use devres-managed allocation for configuration data > drm/msm/mdp5: use devres-managed allocation for CTL manager data > drm/msm/mdp5: use devres-managed allocation for mixer data > drm/msm/mdp5: use devres-managed allocation for pipe data > drm/msm/mdp5: use devres-managed allocation for SMP data > drm/msm/mdp5: use devres-managed allocation for INTF data > drm/msm/mdp5: use drmm-managed allocation for mdp5_crtc > drm/msm/mdp5: use drmm-managed allocation for mdp5_encoder > drm/msm/mdp5: use drmm-managed allocation for mdp5_plane > drm/msm/mdp4: use bulk regulators API for LCDC encoder > drm/msm/mdp4: use drmm-managed allocation for mdp4_crtc > drm/msm/mdp4: use drmm-managed allocation for mdp4_dsi_encoder > drm/msm/mdp4: use drmm-managed allocation for mdp4_dtv_encoder > drm/msm/mdp4: use drmm-managed allocation for mdp4_lcdc_encoder > drm/msm/mdp4: use drmm-managed allocation for mdp4_plane > drm/msm: drop mdp_get_formats() > > [...] Applied, thanks! [02/17] drm/msm/mdp5: use devres-managed allocation for configuration data https://gitlab.freedesktop.org/lumag/msm/-/commit/062aeadeba1d [03/17] drm/msm/mdp5: use devres-managed allocation for CTL manager data https://gitlab.freedesktop.org/lumag/msm/-/commit/4c1f4c1f1b43 [04/17] drm/msm/mdp5: use devres-managed allocation for mixer data https://gitlab.freedesktop.org/lumag/msm/-/commit/1ad175c2c884 [05/17] drm/msm/mdp5: use devres-managed allocation for pipe data https://gitlab.freedesktop.org/lumag/msm/-/commit/323e9a18d6e1 [06/17] drm/msm/mdp5: use devres-managed allocation for SMP data https://gitlab.freedesktop.org/lumag/msm/-/commit/531d5313d934 [07/17] drm/msm/mdp5: use devres-managed allocation for INTF data https://gitlab.freedesktop.org/lumag/msm/-/commit/6de8288bf668 [08/17] drm/msm/mdp5: use drmm-managed allocation for mdp5_crtc https://gitlab.freedesktop.org/lumag/msm/-/commit/6f235e3d6b18 [09/17] drm/msm/mdp5: use drmm-managed allocation for mdp5_encoder https://gitlab.freedesktop.org/lumag/msm/-/commit/669afee4a17e [11/17] drm/msm/mdp4: use bulk regulators API for LCDC encoder https://gitlab.freedesktop.org/lumag/msm/-/commit/54f1fbcb47d4 [12/17] drm/msm/mdp4: use drmm-managed allocation for mdp4_crtc https://gitlab.freedesktop.org/lumag/msm/-/commit/783ad6e6312f [13/17] drm/msm/mdp4: use drmm-managed allocation for mdp4_dsi_encoder https://gitlab.freedesktop.org/lumag/msm/-/commit/e79571e8708b [14/17] drm/msm/mdp4: use drmm-managed allocation for mdp4_dtv_encoder https://gitlab.freedesktop.org/lumag/msm/-/commit/93d6e1b82b93 [15/17] drm/msm/mdp4: use drmm-managed allocation for mdp4_lcdc_encoder https://gitlab.freedesktop.org/lumag/msm/-/commit/2c24668cc068 Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm/a6xx: add QMP dependency
On Mon, 16 Oct 2023 22:04:03 +0200, Arnd Bergmann wrote: > When QMP is in a loadable module, the A6xx GPU driver fails to link > as built-in: > > x86_64-linux-ld: drivers/gpu/drm/msm/adreno/a6xx_gmu.o: in function > `a6xx_gmu_resume': > a6xx_gmu.c:(.text+0xd62): undefined reference to `qmp_send' > > Add the usual dependency that still allows compiling without QMP but > otherwise avoids the broken combination of options. > > [...] Applied, thanks! [1/1] drm/msm/a6xx: add QMP dependency https://gitlab.freedesktop.org/lumag/msm/-/commit/96ab215b2d5e Best regards, -- Dmitry Baryshkov
Re: [PATCH 1/2] dt-bindings: display: simple: Add boe,bp082wx1-100 8.2" panel
On Sat, Dec 02, 2023 at 10:11:52AM +0200, Tony Lindgren wrote: > This panel is found on Motorola mapphone tablets mz607 to mz609. Acked-by: Conor Dooley Cheers, Conor. > > Signed-off-by: Tony Lindgren > --- > .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > @@ -73,6 +73,8 @@ properties: >- auo,t215hvn01 > # Shanghai AVIC Optoelectronics 7" 1024x600 color TFT-LCD panel >- avic,tm070ddh03 > +# BOE BP082WX1-100 8.2" WXGA (1280x800) LVDS panel > + - boe,bp082wx1-100 > # BOE BP101WX1-100 10.1" WXGA (1280x800) LVDS panel >- boe,bp101wx1-100 > # BOE EV121WXM-N10-1850 12.1" WXGA (1280x800) TFT LCD panel > -- > 2.43.0 signature.asc Description: PGP signature
Re: [PATCH] driver: gpu: Fix warning directly dereferencing a rcu pointer
On 11/30/23 05:22, Danilo Krummrich wrote:
Hi Abhinav,
Thanks for sending this follow-up patch.
On 11/26/23 15:57, Abhinav Singh wrote:
Fix a sparse warning with this message
"warning:dereference of noderef expression". In this context it means we
are dereferencing a __rcu tagged pointer directly.
We should not be directly dereferencing a rcu pointer. To get a normal
(non __rcu tagged pointer) from a __rcu tagged pointer we are using the
function unrcu_pointer(...). The non __rcu tagged pointer then can be
dereferenced just like a normal pointer.
Can you please add a brief explanation why unrcu_pointer() is fine here?
Is this description okay
"The reason for using unrcu_pointer(...) instead of rcu_dereference(...)
or rcu_dereference_protected(...) is because, before nv10_fence_emit()
and nv_04_fence_emit() did not add this fence to the fence context's
pending list, thus channel doesn't need any protection" ?
I tested with qemu with this command
qemu-system-x86_64 \
-m 2G \
-smp 2 \
-kernel bzImage \
-append "console=ttyS0 root=/dev/sda earlyprintk=serial
net.ifnames=0" \
-drive file=bullseye.img,format=raw \
-net user,host=10.0.2.10,hostfwd=tcp:127.0.0.1:10021-:22 \
-net nic,model=e1000 \
-enable-kvm \
-nographic \
-pidfile vm.pid \
2>&1 | tee vm.log
with lockdep enabled.
How is that relevant for this patch?
- Danilo
To test rcu related code lockdep must be enabled, it gives any warning
or error message if we are dealing inappropriately with rcu pointers. So
I tested this lockdep enabled. I added the test description in this
patch as well
https://lore.kernel.org/all/0754e669-8b00-461c-b6fe-79c659bf5...@redhat.com/
which is very similar to this patch so I thought I should here as well.
Is it not relevant here?
Thank You,
Abhinav Singh
Signed-off-by: Abhinav Singh
---
drivers/gpu/drm/nouveau/nv10_fence.c | 2 +-
drivers/gpu/drm/nouveau/nv84_fence.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nv10_fence.c
b/drivers/gpu/drm/nouveau/nv10_fence.c
index c6a0db5b9e21..845b64c079ed 100644
--- a/drivers/gpu/drm/nouveau/nv10_fence.c
+++ b/drivers/gpu/drm/nouveau/nv10_fence.c
@@ -32,7 +32,7 @@
int
nv10_fence_emit(struct nouveau_fence *fence)
{
- struct nvif_push *push = fence->channel->chan.push;
+ struct nvif_push *push = unrcu_pointer(fence->channel)->chan.push;
int ret = PUSH_WAIT(push, 2);
if (ret == 0) {
PUSH_MTHD(push, NV06E, SET_REFERENCE, fence->base.seqno);
diff --git a/drivers/gpu/drm/nouveau/nv84_fence.c
b/drivers/gpu/drm/nouveau/nv84_fence.c
index 812b8c62eeba..d42e72e23dec 100644
--- a/drivers/gpu/drm/nouveau/nv84_fence.c
+++ b/drivers/gpu/drm/nouveau/nv84_fence.c
@@ -85,7 +85,7 @@ nv84_fence_chid(struct nouveau_channel *chan)
static int
nv84_fence_emit(struct nouveau_fence *fence)
{
- struct nouveau_channel *chan = fence->channel;
+ struct nouveau_channel *chan = unrcu_pointer(fence->channel);
struct nv84_fence_chan *fctx = chan->fence;
u64 addr = fctx->vma->addr + nv84_fence_chid(chan) * 16;
