[PATCH] drm/amd: fix potential memory leak
This patch fix potential memory leak (clk_src) when function run into last return NULL. Signed-off-by: Bernard Zhao --- drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c b/drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c index 85f32206a766..c7bb76a2a8c2 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c @@ -1643,6 +1643,7 @@ static struct clock_source *dcn31_clock_source_create( } BREAK_TO_DEBUGGER(); + kfree(clk_src); return NULL; } -- 2.33.1
Re: [PATCH v2 4/7] regulator: core: Allow specifying an initial load w/ the bulk API
Hi, Douglas Just an update on the fix you pointed out previously here: > > [1] > > https://lore.kernel.org/r/20220809142738.1.I91625242f137c707bb345c51c80c5ecee02eeff3@changeid With it I could boot the hikey960 build to the home screen if it does not use the GKI kernel. but the problem will be reproduced if it uses the GKI kernel. And if this change is reverted, then it could boot with the GKI kernel as well. I am not sure what's the reason there, but there seems to be some difference with the fix above and the workaround of revert. Not sure if you have any idea about that. Regarding the GKI kernel(Android Generic Kernel Image)[2], it's built from the android-mainline tree(f51334eac4de) without any workaround. (Neither the revert, nor the fix applied), and the regulator modules used for the hikey960 build are hi6421v530-regulator.ko and hi655x-regulator.ko I am still not sure if it would work with the GKI kernel that has the fix that you pointed out in. the case that both the GKI kernel and vendor tree have the fix. Will update here when I have some results. [2]: https://source.android.com/docs/core/architecture/kernel/generic-kernel-image?hl=en Thanks, Yongqin Liu On Thu, 18 Aug 2022 at 00:52, Yongqin Liu wrote: > > Hi, Douglas > > On Wed, 17 Aug 2022 at 22:26, Doug Anderson wrote: > > > > Hi, > > > > On Tue, Aug 16, 2022 at 5:58 AM Yongqin Liu wrote: > > > > > > HI, Douglas > > > > > > With this change, I get one kernel panic with my hikey960 > > > android-mainline based Android build, > > > if it's reverted, then the build could boot to the home screen > > > successfully. > > > From the log information I shared here, not sure if you have any idea > > > what I could do to have the hikey960 > > > build work with this change, > > > > > > The kernel panic is something like this, for details, please check the > > > log here: http://ix.io/47Lq > > > > > > [ 10.738042][ T193] adv7511 1-0039: error : Failed > > > to get supply 'v1p2' > > > [ 10.748457][ T194] apexd: Found pre-installed APEX > > > /system/apex/com.android.os.statsd.apex > > > [ 10.752908][ T67] Unable to handle kernel read from unreadable > > > memory at virtual address 004c > > > [ 10.753116][T8] Unable to handle kernel read from unreadable > > > memory at virtual address 0078 > > > [ 10.753130][T8] Mem abort info: > > > [ 10.753135][T8] ESR = 0x9605 > > > [ 10.753142][T8] EC = 0x25: DABT (current EL), IL = 32 bits > > > [ 10.753152][T8] SET = 0, FnV = 0 > > > [ 10.753159][T8] EA = 0, S1PTW = 0 > > > [ 10.753166][T8] FSC = 0x05: level 1 translation fault > > > [ 10.753174][T8] Data abort info: > > > [ 10.753179][T8] ISV = 0, ISS = 0x0005 > > > [ 10.753184][T8] CM = 0, WnR = 0 > > > [ 10.753192][T8] user pgtable: 4k pages, 39-bit VAs, > > > pgdp=03098000 > > > [ 10.753204][T8] [0078] pgd=, > > > p4d=, pud= > > > [ 10.753232][T8] Internal error: Oops: 9605 [#1] PREEMPT SMP > > > [ 10.753245][T8] Modules linked in: adv7511(E+) tcpci_rt1711h(E) > > > hci_uart(E) btqca(E) btbcm(E) cpufreq_dt(E) hi3660_i2s(E) > > > hisi_hikey_usb(E) hi6421_pmic_core(E) syscon_reboot_mode(E) > > > reboot_mode(E) hi3660_mailbox(E) dw_mmc_k3(E) hisi_thermal(E) > > > dw_mmc_pltfm(E) dw_mmc(E) kirin_drm(E) snd_soc_simple_card(E) > > > snd_soc_simple_card_utils(E) spi_pl022(E) kirin_dsi(E) > > > phy_hi3660_usb3(E) i2c_designware_platform(E) drm_cma_helper(E) > > > i2c_designware_core(E) mali_kbase(OE) k3dma(E) cma_heap(E) > > > system_heap(E) > > > [ 10.753436][T8] CPU: 2 PID: 8 Comm: kworker/u16:0 Tainted: G > > >OE 5.19.0-mainline-03487-g86d047950300-dirty #1 > > > [ 10.753454][T8] Hardware name: HiKey960 (DT) > > > [ 10.753463][T8] Workqueue: events_unbound async_run_entry_fn > > > [ 10.753497][T8] pstate: 6045 (nZCv daif +PAN -UAO -TCO -DIT > > > -SSBS BTYPE=--) > > > [ 10.753516][T8] pc : regulator_bulk_enable_async+0x3c/0x98 > > > [ 10.753540][T8] lr : async_run_entry_fn+0x30/0xf8 > > > [ 10.753559][T8] sp : ffc009ed3d20 > > > [ 10.753567][T8] x29: ffc009ed3d40 x28: 0402 > > > x27: ff801ad99828 > > > [ 10.753592][T8] x26: ff803217b010 x25: 0002 > > > x24: ff8003385da8 > > > [ 10.753617][T8] x23: ff8003385da0 x22: ff801ad94805 > > > x21: ff8003385da0 > > > [ 10.753642][T8] x20: x19: ff8003143d20 > > > x18: ffc008075028 > > > [ 10.753667][T8] x17: 00040044 x16: 0001 > > > x15: 0010 > > > [ 10.753691][T8] x14: x13: 0f58 > > > x12: 8235 > > > [ 10.753715][T8] x11: 6acfbfa2f400 x10: 0016 x9 > > > : 00ff > > > [ 10.753740][T8] x8 : da9ecda1b6
Re: [PATCH 2/2] [WIP]: media: Add Synaptics compressed tiled format
On Sat, Aug 20, 2022 at 12:44 AM Hsia-Jun Li wrote:
>
>
>
> On 8/19/22 23:28, Nicolas Dufresne wrote:
> > CAUTION: Email originated externally, do not click links or open
> > attachments unless you recognize the sender and know the content is safe.
> >
> >
> > Le vendredi 19 août 2022 à 02:13 +0300, Laurent Pinchart a écrit :
> >> On Thu, Aug 18, 2022 at 02:33:42PM +0800, Hsia-Jun Li wrote:
> >>> On 8/18/22 14:06, Tomasz Figa wrote:
> On Tue, Aug 9, 2022 at 1:28 AM Hsia-Jun Li
> wrote:
> >
> > From: "Hsia-Jun(Randy) Li"
> >
> > The most of detail has been written in the drm.
> >>
> >> This patch still needs a description of the format, which should go to
> >> Documentation/userspace-api/media/v4l/.
> >>
> > Please notice that the tiled formats here request
> > one more plane for storing the motion vector metadata.
> > This buffer won't be compressed, so you can't append
> > it to luma or chroma plane.
>
> Does the motion vector buffer need to be exposed to userspace? Is the
> decoder stateless (requires userspace to specify the reference frames)
> or stateful (manages the entire decoding process internally)?
> >>>
> >>> No, users don't need to access them at all. Just they need a different
> >>> dma-heap.
> >>>
> >>> You would only get the stateful version of both encoder and decoder.
> >>
> >> Shouldn't the motion vectors be stored in a separate V4L2 buffer,
> >> submitted through a different queue then ?
> >
> > Imho, I believe these should be invisible to users and pooled separately to
> > reduce the overhead. The number of reference is usually lower then the
> > number of
> > allocated display buffers.
> >
> You can't. The motion vector buffer can't share with the luma and chroma
> data planes, nor the data plane for the compression meta data.
I believe what Nicolas is suggesting is to just keep the MV buffer
handling completely separate from video buffers. Just keep a map
between frame buffer and MV buffer in the driver and use the right
buffer when triggering a decode.
>
> You could consider this as a security requirement(the memory region for
> the MV could only be accessed by the decoder) or hardware limitation.
>
> It is also not very easy to manage such a large buffer that would change
> when the resolution changed.
How does it differ from managing additional planes of video buffers?
Best regards,
Tomasz
> >>
> > Signed-off-by: Hsia-Jun(Randy) Li
> > ---
> >drivers/media/v4l2-core/v4l2-common.c | 1 +
> >drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
> >include/uapi/linux/videodev2.h| 2 ++
> >3 files changed, 5 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c
> > b/drivers/media/v4l2-core/v4l2-common.c
> > index e0fbe6ba4b6c..f645278b3055 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -314,6 +314,7 @@ const struct v4l2_format_info *v4l2_format_info(u32
> > format)
> > { .format = V4L2_PIX_FMT_SGBRG12, .pixel_enc =
> > V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0,
> > 0, 0 }, .hdiv = 1, .vdiv = 1 },
> > { .format = V4L2_PIX_FMT_SGRBG12, .pixel_enc =
> > V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0,
> > 0, 0 }, .hdiv = 1, .vdiv = 1 },
> > { .format = V4L2_PIX_FMT_SRGGB12, .pixel_enc =
> > V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0,
> > 0, 0 }, .hdiv = 1, .vdiv = 1 },
> > + { .format = V4L2_PIX_FMT_NV12M_V4H1C, .pixel_enc =
> > V4L2_PIXEL_ENC_YUV, .mem_planes = 5, .comp_planes = 2, .bpp = { 1, 2,
> > 0, 0 }, .hdiv = 2, .vdiv = 2, .block_w = { 128, 128 }, .block_h = {
> > 128, 128 } },
> > };
> > unsigned int i;
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index e6fd355a2e92..8f65964aff08 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1497,6 +1497,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc
> > *fmt)
> > case V4L2_PIX_FMT_MT21C:descr = "Mediatek
> > Compressed Format"; break;
> > case V4L2_PIX_FMT_QC08C:descr = "QCOM
> > Compressed 8-bit Format"; break;
> > case V4L2_PIX_FMT_QC10C:descr = "QCOM
> > Compressed 10-bit Format"; break;
> > + case V4L2_PIX_FMT_NV12M_V4H1C: descr = "Synaptics
> > Compressed 8-bit tiled Format";break;
> > + case V4L2_PIX_FMT_NV12M_10_V4H3P8C: descr =
> > "Synaptics Compressed 10-bit tiled Format";break;
> > default:
> > i
Re: [PATCH] drm: Move radeon and amdgpu Kconfig options into their directories
Am 23.08.22 um 02:01 schrieb Andrew Davis: Most Kconfig options to enable a driver are in the Kconfig file inside the relevant directory, move these two to the same. Signed-off-by: Andrew Davis With the tab vs. spaces pointed out by Randy fixed the patch is Reviewed-by: Christian König --- drivers/gpu/drm/Kconfig| 42 -- drivers/gpu/drm/amd/amdgpu/Kconfig | 22 drivers/gpu/drm/radeon/Kconfig | 22 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 6c2256e8474b..24fa9ccd92a4 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -234,50 +234,8 @@ source "drivers/gpu/drm/i2c/Kconfig" source "drivers/gpu/drm/arm/Kconfig" -config DRM_RADEON - tristate "ATI Radeon" - depends on DRM && PCI && MMU - depends on AGP || !AGP - select FW_LOADER - select DRM_DISPLAY_DP_HELPER - select DRM_DISPLAY_HELPER -select DRM_KMS_HELPER -select DRM_TTM - select DRM_TTM_HELPER - select POWER_SUPPLY - select HWMON - select BACKLIGHT_CLASS_DEVICE - select INTERVAL_TREE - help - Choose this option if you have an ATI Radeon graphics card. There - are both PCI and AGP versions. You don't need to choose this to - run the Radeon in plain VGA mode. - - If M is selected, the module will be called radeon. - source "drivers/gpu/drm/radeon/Kconfig" -config DRM_AMDGPU - tristate "AMD GPU" - depends on DRM && PCI && MMU - select FW_LOADER - select DRM_DISPLAY_DP_HELPER - select DRM_DISPLAY_HDMI_HELPER - select DRM_DISPLAY_HELPER - select DRM_KMS_HELPER - select DRM_SCHED - select DRM_TTM - select DRM_TTM_HELPER - select POWER_SUPPLY - select HWMON - select BACKLIGHT_CLASS_DEVICE - select INTERVAL_TREE - select DRM_BUDDY - help - Choose this option if you have a recent AMD Radeon graphics card. - - If M is selected, the module will be called amdgpu. - source "drivers/gpu/drm/amd/amdgpu/Kconfig" source "drivers/gpu/drm/nouveau/Kconfig" diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index d55275de..36b1206124cf 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -1,4 +1,26 @@ # SPDX-License-Identifier: MIT + +config DRM_AMDGPU + tristate "AMD GPU" + depends on DRM && PCI && MMU + select FW_LOADER + select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HDMI_HELPER + select DRM_DISPLAY_HELPER + select DRM_KMS_HELPER + select DRM_SCHED + select DRM_TTM + select DRM_TTM_HELPER + select POWER_SUPPLY + select HWMON + select BACKLIGHT_CLASS_DEVICE + select INTERVAL_TREE + select DRM_BUDDY + help + Choose this option if you have a recent AMD Radeon graphics card. + + If M is selected, the module will be called amdgpu. + config DRM_AMDGPU_SI bool "Enable amdgpu support for SI parts" depends on DRM_AMDGPU diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig index 52819e7f1fca..3248d12c562d 100644 --- a/drivers/gpu/drm/radeon/Kconfig +++ b/drivers/gpu/drm/radeon/Kconfig @@ -1,4 +1,26 @@ # SPDX-License-Identifier: MIT + +config DRM_RADEON + tristate "ATI Radeon" + depends on DRM && PCI && MMU + depends on AGP || !AGP + select FW_LOADER + select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HELPER +select DRM_KMS_HELPER +select DRM_TTM + select DRM_TTM_HELPER + select POWER_SUPPLY + select HWMON + select BACKLIGHT_CLASS_DEVICE + select INTERVAL_TREE + help + Choose this option if you have an ATI Radeon graphics card. There + are both PCI and AGP versions. You don't need to choose this to + run the Radeon in plain VGA mode. + + If M is selected, the module will be called radeon. + config DRM_RADEON_USERPTR bool "Always enable userptr support" depends on DRM_RADEON
Re: [PATCH v3 3/3] dt-bindings: msm/dp: handle DP vs eDP difference
Quoting Dmitry Baryshkov (2022-08-22 11:49:00) > The #sound-dai-cells property should be used only for DP controllers. It > doesn't make sense for eDP, there is no support for audio output. The > aux-bus should not be used for DP controllers. Also p1 MMIO region > should be used only for DP controllers. > > Take care of these differences. > > Reviewed-by: Rob Herring > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [PATCH v3 2/3] dt-bindings: msm/dp: add missing properties
Quoting Dmitry Baryshkov (2022-08-22 11:48:59) > Document missing definitions for opp-table (DP controller OPPs), aux-bus > (DP AUX BUS) and data-lanes (DP/eDP lanes mapping) properties. > > Reviewed-by: Stephen Boyd > Acked-by: Krzysztof Kozlowski > Signed-off-by: Dmitry Baryshkov > --- > .../bindings/display/msm/dp-controller.yaml | 12 > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > index 391910d91e43..52cbf00df0ba 100644 > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml > @@ -70,9 +70,21 @@ properties: >operating-points-v2: > maxItems: 1 > > + opp-table: true > + >power-domains: > maxItems: 1 > > + aux-bus: > +$ref: /schemas/display/dp-aux-bus.yaml# > + > + data-lanes: > +$ref: /schemas/types.yaml#/definitions/uint32-array > +minItems: 1 > +maxItems: 4 > +items: > + maximum: 3 It should be marked deprecated, right?
Re: [PATCH v3 1/3] dt-bindings: msm/dp: mark vdda supplies as deprecated
Quoting Dmitry Baryshkov (2022-08-22 11:48:58)
> The commit 85936d4f3815 ("phy: qcom-qmp: add regulator_set_load to dp
> phy") moved setting regulator load to the DP PHY driver (QMP). Then, the
> commit 7516351bebc1 ("drm/msm/dp: delete vdda regulator related
> functions from eDP/DP controller") removed support for VDDA supplies
> from the DP controller driver.
> Mark these properties as deprecated and drop them from the example.
>
> Acked-by: Rob Herring
> Signed-off-by: Dmitry Baryshkov
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH] drm/msm/dpu: drop unused variable from dpu_kms_mdp_snapshot()
Quoting Dmitry Baryshkov (2022-08-22 10:22:04)
> Follow up the merge of address fields and drop the variable that became
> unused after the commit 9403f9a42c88 ("drm/msm/dpu: merge base_off with
> blk_off in struct dpu_hw_blk_reg_map").
>
> Fixes: 9403f9a42c88 ("drm/msm/dpu: merge base_off with blk_off in struct
> dpu_hw_blk_reg_map")
> Signed-off-by: Dmitry Baryshkov
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH] drm/msm/dpu: drop unused memory allocation
Quoting Dmitry Baryshkov (2022-08-22 10:24:55)
> Drop the dpu_cfg variable and corresponding kzalloc, which became unused
> after changing hw catalog to static configuration.
>
> Fixes: de7d480f5e8c ("drm/msm/dpu: make dpu hardware catalog static const")
> Reported-by: kernel test robot
> Reported-by: Dan Carpenter
> Signed-off-by: Dmitry Baryshkov
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH v2 8/9] dt-bindings: msm/dp: add missing properties
Quoting Dmitry Baryshkov (2022-08-22 11:46:39) > On 12/07/2022 02:16, Rob Herring wrote: > > > > But this is the wrong location for 'data-lanes'. It belongs in an > > endpoint node. > > I rechecked the existing device trees (sc7280-herobrine.dtsi). The > data-lanes are already inside the main dp controller node. I'll take a > glance on fixing the driver to check the dp_out endpoint (and update > existing DT to move the property to the endpoint node), but to make > schema compatible with the existing device trees we'd probably still > need this property (which can be marked as deprecated once the proper > endpoint property is supported). Does that sound plausible? It would be nice if drm_of_get_data_lanes_count() took some port and endpoint number instead of a node pointer directly. Then you couldn't mess this up as easily.
Re: [PATCH] drm/panfrost: Update io-pgtable API
> -static size_t get_pgsize(u64 addr, size_t size)
> +static size_t get_pgsize(u64 addr, size_t size, size_t *count)
> {
> - if (addr & (SZ_2M - 1) || size < SZ_2M)
> - return SZ_4K;
> + size_t blk_offset = -addr % SZ_2M;
addr is unsigned. if this is correct, it's magic.
[PATCH] drm/msm/dp: Silence inconsistent indent warning
Build robots complain
smatch warnings:
drivers/gpu/drm/msm/dp/dp_link.c:969 dp_link_process_link_status_update()
warn: inconsistent indenting
Fix it.
Cc: Kuogee Hsieh
Fixes: ea530388e64b ("drm/msm/dp: skip checking LINK_STATUS_UPDATED bit")
Reported-by: kernel test robot
Signed-off-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_link.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 36f0af02749f..1620110806cf 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -965,8 +965,7 @@ static int dp_link_process_link_status_update(struct
dp_link_private *link)
if (channel_eq_done && clock_recovery_done)
return -EINVAL;
-
- return 0;
+ return 0;
}
/**
base-commit: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
--
https://chromeos.dev
Re: [PATCH v7 1/8] overflow: Move and add few utility macros into overflow
On 8/23/22 5:12 AM, Kees Cook wrote:
On Tue, Aug 23, 2022 at 04:32:10AM +0900, Gwan-gyeong Mun wrote:
On 8/22/22 11:05 PM, Andrzej Hajda wrote:
On 18.08.2022 02:12, Kees Cook wrote:
On Thu, Aug 18, 2022 at 01:07:29AM +0200, Andi Shyti wrote:
[...]
+#define safe_conversion(ptr, value) ({ \
+ typeof(value) __v = (value); \
+ typeof(ptr) __ptr = (ptr); \
+ overflows_type(__v, *__ptr) ? 0 : ((*__ptr =
(typeof(*__ptr))__v), 1); \
+})
I try to avoid "safe" as an adjective for interface names, since it
doesn't really answer "safe from what?" This looks more like "assign, but
zero when out of bounds". And it can be built from existing macros here:
if (check_add_overflow(0, value, ptr))
*ptr = 0;
I actually want to push back on this a bit, because there can still be
logic bugs built around this kind of primitive. Shouldn't out-of-bounds
assignments be seen as a direct failure? I would think this would be
sufficient:
#define check_assign(value, ptr) check_add_overflow(0, value, ptr)
And callers would do:
if (check_assign(value, &var))
return -EINVAL;
Yes, I also like check_assign() you suggested more than safe_conversion.
As shown below, it would be more readable to return true when assign
succeeds and false when it fails. What do you think?
No, this inverts the style of all the other check_*() functions, so it
should remain "non-zero is failure".
Hi Kees,
Yes, I will not invert this part as you commented.
/**
* check_assign - perform a type conversion (cast) of an source value into
* a new variable, checking that the destination is large enough to hold the
* source value.
*
* @value: Source value
* @ptr: Destination pointer address, If the pointer type is not used, a
warning message is output during build.
*
* Returns:
* If the value would overflow the destination, it returns false. If not
return true.
*/
#define check_assign(value, ptr) __must_check_overflow(({ \
typecheck_pointer(ptr); \
!__builtin_add_overflow(0, value, ptr); \
}))
Please don't use the __builtin*s, instead stick to the check_* family,
as they correctly wrap the builtins and perform type checking, etc. As
mentioned, check_assign() should just be:
#define check_assign(value, ptr) check_add_overflow(0, value, ptr)
I don't think any of the other code is needed? What's the use-case for
the other stuff? i.e. Why does anything need overflows_type()?
And, the reason for using the __builtin_add_overflow() built-in function
directly instead of using the check_add_overflow() function is ,
#define check_add_overflow(a, b, d) __must_check_overflow(({\
typeof(a) __a = (a);\
typeof(b) __b = (b);\
typeof(d) __d = (d);\
(void) (&__a == &__b); \
(void) (&__a == __d); \
__builtin_add_overflow(__a, __b, __d); \
}))
In this part of the implementation of check_add_overflow()
(void) (&__a == &__b);
(void) (&__a == __d);
When comparing the pointer types of a, b, and d, if the pointer types of
source and ptr in check_assign() are different, a warning may occur when
building, I used the __builtin_add_overflow() built-in function directly.
Br,
G.G.
-Kees
Re: [PATCH -next] drm/hisilicon/hibmc: Fix COMPILE_TEST building without MMU
On 8/22/22 19:09, YueHaibing wrote:
> WARNING: unmet direct dependencies detected for DRM_TTM
> Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && MMU [=n]
> Selected by [y]:
> - DRM_TTM_HELPER [=y] && HAS_IOMEM [=y] && DRM [=y]
> - DRM_HISI_HIBMC [=y] && HAS_IOMEM [=y] && DRM [=y] && PCI [=y] && (ARM64
> || COMPILE_TEST [=y])
>
> Add missing MMU dependency to fix this.
>
> Fixes: a0f25a6bb319 ("drm/hisilicon/hibmc: Allow to be built if COMPILE_TEST
> is enabled")
> Signed-off-by: YueHaibing
> ---
> drivers/gpu/drm/hisilicon/hibmc/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> index 073adfe438dd..e5ef1b573732 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> config DRM_HISI_HIBMC
> tristate "DRM Support for Hisilicon Hibmc"
> - depends on DRM && PCI && (ARM64 || COMPILE_TEST)
> + depends on DRM && PCI && (ARM64 || COMPILE_TEST) && MMU
> select DRM_KMS_HELPER
> select DRM_VRAM_HELPER
> select DRM_TTM
Yes, or this one from April 8, 2022:
https://lore.kernel.org/lkml/20220409030504.16089-1-rdun...@infradead.org/
thanks.
--
~Randy
Re: [PATCH -next] drm/bridge: Add missing clk_disable_unprepare() in analogix_dp_resume()
Hi, On Wed, Aug 17, 2022 at 05:05:25PM -0700, Brian Norris wrote: > Hmm, actually I'm going to have to retract that now that I've given it > some more testing locally. I happen to have a system where I commonly > hit this error case, and I'm thinking commit 211f276ed3d9 is actually > wrong, and so we shouldn't be "fixing" its error handling -- we should > be reverting it. I've submitted that for review here: https://lore.kernel.org/all/20220822180729.1.I8ac5abe3a4c1c6fd5c061686c6e883c22f69022c@changeid/ [PATCH] Revert "drm: bridge: analogix/dp: add panel prepare/unprepare in suspend/resume time" I'd appreciate your review/testing. (NB: I failed to honor the .mailmap for Andrzej Hajda.) > Now separately, I have to figure out why I'm hitting this error case in > the first place... FWIW, I captured the reason in point 3 on the above Revert. The pm_runtime_*() handling in the panel driver fails (-EACCES) because the bridge driver is resuming before the panel. (The DRM suspend/resume helpers handle things in the correct order.) This problem is also resolved by simply reverting. Brian
Re: [PATCH] dma-buf: cma_heap: Check for device max segment size when attaching
Hi Andrew,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.0-rc2 next-20220822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Andrew-Davis/dma-buf-cma_heap-Check-for-device-max-segment-size-when-attaching/20220823-073240
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
config: s390-allyesconfig
(https://download.01.org/0day-ci/archive/20220823/202208230840.nplcmvvn-...@intel.com/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/f390cef50ba6681ea767283e413cb8e9f8f2b426
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Andrew-Davis/dma-buf-cma_heap-Check-for-device-max-segment-size-when-attaching/20220823-073240
git checkout f390cef50ba6681ea767283e413cb8e9f8f2b426
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=s390 SHELL=/bin/bash drivers/dma-buf/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/dma-buf/heaps/cma_heap.c: In function 'cma_heap_attach':
>> drivers/dma-buf/heaps/cma_heap.c:61:9: warning: ISO C90 forbids mixed
>> declarations and code [-Wdeclaration-after-statement]
61 | size_t max_segment = dma_get_max_seg_size(attachment->dev);
| ^~
vim +61 drivers/dma-buf/heaps/cma_heap.c
49
50 static int cma_heap_attach(struct dma_buf *dmabuf,
51 struct dma_buf_attachment *attachment)
52 {
53 struct cma_heap_buffer *buffer = dmabuf->priv;
54 struct dma_heap_attachment *a;
55 int ret;
56
57 a = kzalloc(sizeof(*a), GFP_KERNEL);
58 if (!a)
59 return -ENOMEM;
60
> 61 size_t max_segment = dma_get_max_seg_size(attachment->dev);
62 ret = sg_alloc_table_from_pages_segment(&a->table,
buffer->pages,
63 buffer->pagecount, 0,
64 buffer->pagecount <<
PAGE_SHIFT,
65 max_segment,
GFP_KERNEL);
66 if (ret) {
67 kfree(a);
68 return ret;
69 }
70
71 a->dev = attachment->dev;
72 INIT_LIST_HEAD(&a->list);
73 a->mapped = false;
74
75 attachment->priv = a;
76
77 mutex_lock(&buffer->lock);
78 list_add(&a->list, &buffer->attachments);
79 mutex_unlock(&buffer->lock);
80
81 return 0;
82 }
83
--
0-DAY CI Kernel Test Service
https://01.org/lkp
[PATCH 2/2] drm: fix drm_mipi_dbi build errors
drm_mipi_dbi needs lots of DRM_KMS_HELPER support, so select
that Kconfig symbol like it is done is most other uses, and
the way that it was before MIPS_DBI was moved from tinydrm
to its core location.
Fixes these build errors:
ld: drivers/gpu/drm/drm_mipi_dbi.o: in function `mipi_dbi_buf_copy':
drivers/gpu/drm/drm_mipi_dbi.c:205: undefined reference to `drm_gem_fb_get_obj'
ld: drivers/gpu/drm/drm_mipi_dbi.c:211: undefined reference to
`drm_gem_fb_begin_cpu_access'
ld: drivers/gpu/drm/drm_mipi_dbi.c:215: undefined reference to `drm_gem_fb_vmap'
ld: drivers/gpu/drm/drm_mipi_dbi.c:222: undefined reference to `drm_fb_swab'
ld: drivers/gpu/drm/drm_mipi_dbi.c:224: undefined reference to `drm_fb_memcpy'
ld: drivers/gpu/drm/drm_mipi_dbi.c:227: undefined reference to
`drm_fb_xrgb_to_rgb565'
ld: drivers/gpu/drm/drm_mipi_dbi.c:235: undefined reference to
`drm_gem_fb_vunmap'
ld: drivers/gpu/drm/drm_mipi_dbi.c:237: undefined reference to
`drm_gem_fb_end_cpu_access'
ld: drivers/gpu/drm/drm_mipi_dbi.o: in function
`mipi_dbi_dev_init_with_formats':
ld: drivers/gpu/drm/drm_mipi_dbi.o:/X64/../drivers/gpu/drm/drm_mipi_dbi.c:469:
undefined reference to `drm_gem_fb_create_with_dirty'
Fixes: 174102f4de23 ("drm/tinydrm: Move mipi-dbi")
Signed-off-by: Randy Dunlap
Reported-by: kernel test robot
Cc: Dillon Min
Cc: Linus Walleij
Cc: Sam Ravnborg
Cc: Noralf Trønnes
Cc: Thomas Zimmermann
Cc: Thierry Reding
Cc: dri-devel@lists.freedesktop.org
Cc: David Airlie
Cc: Daniel Vetter
---
drivers/gpu/drm/Kconfig |1 +
1 file changed, 1 insertion(+)
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -31,6 +31,7 @@ menuconfig DRM
config DRM_MIPI_DBI
tristate
depends on DRM
+ select DRM_KMS_HELPER
config DRM_MIPI_DSI
bool
[PATCH 1/2] drm/panel: use 'select' for Ili9341 panel driver helpers
Use 'select' instead of 'depends on' for DRM helpers for the
Ilitek ILI9341 panel driver.
This is what is done in the vast majority of other cases and
this makes it possible to fix a build error with drm_mipi_dbi.
Fixes: 5a04227326b0 ("drm/panel: Add ilitek ili9341 panel driver")
Signed-off-by: Randy Dunlap
Cc: Dillon Min
Cc: Linus Walleij
Cc: Sam Ravnborg
Cc: Noralf Trønnes
Cc: Thomas Zimmermann
Cc: Thierry Reding
Cc: dri-devel@lists.freedesktop.org
Cc: David Airlie
Cc: Daniel Vetter
---
drivers/gpu/drm/panel/Kconfig |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -165,8 +165,8 @@ config DRM_PANEL_ILITEK_IL9322
config DRM_PANEL_ILITEK_ILI9341
tristate "Ilitek ILI9341 240x320 QVGA panels"
depends on OF && SPI
- depends on DRM_KMS_HELPER
- depends on DRM_GEM_DMA_HELPER
+ select DRM_KMS_HELPER
+ select DRM_GEM_DMA_HELPER
depends on BACKLIGHT_CLASS_DEVICE
select DRM_MIPI_DBI
help
Re: [PATCH] drm: Move radeon and amdgpu Kconfig options into their directories
On 8/22/22 7:06 PM, Randy Dunlap wrote: Hi-- On 8/22/22 17:01, Andrew Davis wrote: Most Kconfig options to enable a driver are in the Kconfig file inside the relevant directory, move these two to the same. Signed-off-by: Andrew Davis --- drivers/gpu/drm/Kconfig| 42 -- drivers/gpu/drm/amd/amdgpu/Kconfig | 22 drivers/gpu/drm/radeon/Kconfig | 22 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 6c2256e8474b..24fa9ccd92a4 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -234,50 +234,8 @@ source "drivers/gpu/drm/i2c/Kconfig" source "drivers/gpu/drm/arm/Kconfig" -config DRM_RADEON - tristate "ATI Radeon" - depends on DRM && PCI && MMU - depends on AGP || !AGP - select FW_LOADER - select DRM_DISPLAY_DP_HELPER - select DRM_DISPLAY_HELPER -select DRM_KMS_HELPER -select DRM_TTM - select DRM_TTM_HELPER - select POWER_SUPPLY - select HWMON - select BACKLIGHT_CLASS_DEVICE - select INTERVAL_TREE - help - Choose this option if you have an ATI Radeon graphics card. There - are both PCI and AGP versions. You don't need to choose this to - run the Radeon in plain VGA mode. - - If M is selected, the module will be called radeon. - source "drivers/gpu/drm/radeon/Kconfig" -config DRM_AMDGPU - tristate "AMD GPU" - depends on DRM && PCI && MMU - select FW_LOADER - select DRM_DISPLAY_DP_HELPER - select DRM_DISPLAY_HDMI_HELPER - select DRM_DISPLAY_HELPER - select DRM_KMS_HELPER - select DRM_SCHED - select DRM_TTM - select DRM_TTM_HELPER - select POWER_SUPPLY - select HWMON - select BACKLIGHT_CLASS_DEVICE - select INTERVAL_TREE - select DRM_BUDDY - help - Choose this option if you have a recent AMD Radeon graphics card. - - If M is selected, the module will be called amdgpu. - source "drivers/gpu/drm/amd/amdgpu/Kconfig" source "drivers/gpu/drm/nouveau/Kconfig" diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index d55275de..36b1206124cf 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -1,4 +1,26 @@ # SPDX-License-Identifier: MIT + +config DRM_AMDGPU + tristate "AMD GPU" + depends on DRM && PCI && MMU + select FW_LOADER + select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HDMI_HELPER + select DRM_DISPLAY_HELPER + select DRM_KMS_HELPER + select DRM_SCHED + select DRM_TTM + select DRM_TTM_HELPER + select POWER_SUPPLY + select HWMON + select BACKLIGHT_CLASS_DEVICE + select INTERVAL_TREE + select DRM_BUDDY + help + Choose this option if you have a recent AMD Radeon graphics card. + + If M is selected, the module will be called amdgpu. + config DRM_AMDGPU_SI bool "Enable amdgpu support for SI parts" depends on DRM_AMDGPU diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig index 52819e7f1fca..3248d12c562d 100644 --- a/drivers/gpu/drm/radeon/Kconfig +++ b/drivers/gpu/drm/radeon/Kconfig @@ -1,4 +1,26 @@ # SPDX-License-Identifier: MIT + +config DRM_RADEON + tristate "ATI Radeon" + depends on DRM && PCI && MMU + depends on AGP || !AGP + select FW_LOADER + select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HELPER +select DRM_KMS_HELPER +select DRM_TTM Would you change those 2 lines above to use one tab + 2 spaces for indentation, please? Sure, I just copy/paste exactly as they are in the top level Kconfig, white-space issues and all, to make it easy to see that nothing was changed. I can fix the indent issue in a followup patch if that work better. Whichever works better. Andrew + select DRM_TTM_HELPER + select POWER_SUPPLY + select HWMON + select BACKLIGHT_CLASS_DEVICE + select INTERVAL_TREE + help + Choose this option if you have an ATI Radeon graphics card. There + are both PCI and AGP versions. You don't need to choose this to + run the Radeon in plain VGA mode. + + If M is selected, the module will be called radeon. + config DRM_RADEON_USERPTR bool "Always enable userptr support" depends on DRM_RADEON
[PATCH] drm/tidss: Set max DMA segment size
We have no segment size limitations. Set to unlimited. Signed-off-by: Andrew Davis --- drivers/gpu/drm/tidss/tidss_dispc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c index dd3c6a606ae2..624545e4636c 100644 --- a/drivers/gpu/drm/tidss/tidss_dispc.c +++ b/drivers/gpu/drm/tidss/tidss_dispc.c @@ -2685,6 +2685,7 @@ int dispc_init(struct tidss_device *tidss) if (r) dev_warn(dev, "cannot set DMA masks to 48-bit\n"); } + dma_set_max_seg_size(dev, UINT_MAX); dispc = devm_kzalloc(dev, sizeof(*dispc), GFP_KERNEL); if (!dispc) -- 2.36.1
Re: [PATCH] drm: Move radeon and amdgpu Kconfig options into their directories
Hi-- On 8/22/22 17:01, Andrew Davis wrote: > Most Kconfig options to enable a driver are in the Kconfig file > inside the relevant directory, move these two to the same. > > Signed-off-by: Andrew Davis > --- > drivers/gpu/drm/Kconfig| 42 -- > drivers/gpu/drm/amd/amdgpu/Kconfig | 22 > drivers/gpu/drm/radeon/Kconfig | 22 > 3 files changed, 44 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 6c2256e8474b..24fa9ccd92a4 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -234,50 +234,8 @@ source "drivers/gpu/drm/i2c/Kconfig" > > source "drivers/gpu/drm/arm/Kconfig" > > -config DRM_RADEON > - tristate "ATI Radeon" > - depends on DRM && PCI && MMU > - depends on AGP || !AGP > - select FW_LOADER > - select DRM_DISPLAY_DP_HELPER > - select DRM_DISPLAY_HELPER > -select DRM_KMS_HELPER > -select DRM_TTM > - select DRM_TTM_HELPER > - select POWER_SUPPLY > - select HWMON > - select BACKLIGHT_CLASS_DEVICE > - select INTERVAL_TREE > - help > - Choose this option if you have an ATI Radeon graphics card. There > - are both PCI and AGP versions. You don't need to choose this to > - run the Radeon in plain VGA mode. > - > - If M is selected, the module will be called radeon. > - > source "drivers/gpu/drm/radeon/Kconfig" > > -config DRM_AMDGPU > - tristate "AMD GPU" > - depends on DRM && PCI && MMU > - select FW_LOADER > - select DRM_DISPLAY_DP_HELPER > - select DRM_DISPLAY_HDMI_HELPER > - select DRM_DISPLAY_HELPER > - select DRM_KMS_HELPER > - select DRM_SCHED > - select DRM_TTM > - select DRM_TTM_HELPER > - select POWER_SUPPLY > - select HWMON > - select BACKLIGHT_CLASS_DEVICE > - select INTERVAL_TREE > - select DRM_BUDDY > - help > - Choose this option if you have a recent AMD Radeon graphics card. > - > - If M is selected, the module will be called amdgpu. > - > source "drivers/gpu/drm/amd/amdgpu/Kconfig" > > source "drivers/gpu/drm/nouveau/Kconfig" > diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig > b/drivers/gpu/drm/amd/amdgpu/Kconfig > index d55275de..36b1206124cf 100644 > --- a/drivers/gpu/drm/amd/amdgpu/Kconfig > +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig > @@ -1,4 +1,26 @@ > # SPDX-License-Identifier: MIT > + > +config DRM_AMDGPU > + tristate "AMD GPU" > + depends on DRM && PCI && MMU > + select FW_LOADER > + select DRM_DISPLAY_DP_HELPER > + select DRM_DISPLAY_HDMI_HELPER > + select DRM_DISPLAY_HELPER > + select DRM_KMS_HELPER > + select DRM_SCHED > + select DRM_TTM > + select DRM_TTM_HELPER > + select POWER_SUPPLY > + select HWMON > + select BACKLIGHT_CLASS_DEVICE > + select INTERVAL_TREE > + select DRM_BUDDY > + help > + Choose this option if you have a recent AMD Radeon graphics card. > + > + If M is selected, the module will be called amdgpu. > + > config DRM_AMDGPU_SI > bool "Enable amdgpu support for SI parts" > depends on DRM_AMDGPU > diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig > index 52819e7f1fca..3248d12c562d 100644 > --- a/drivers/gpu/drm/radeon/Kconfig > +++ b/drivers/gpu/drm/radeon/Kconfig > @@ -1,4 +1,26 @@ > # SPDX-License-Identifier: MIT > + > +config DRM_RADEON > + tristate "ATI Radeon" > + depends on DRM && PCI && MMU > + depends on AGP || !AGP > + select FW_LOADER > + select DRM_DISPLAY_DP_HELPER > + select DRM_DISPLAY_HELPER > +select DRM_KMS_HELPER > +select DRM_TTM Would you change those 2 lines above to use one tab + 2 spaces for indentation, please? > + select DRM_TTM_HELPER > + select POWER_SUPPLY > + select HWMON > + select BACKLIGHT_CLASS_DEVICE > + select INTERVAL_TREE > + help > + Choose this option if you have an ATI Radeon graphics card. There > + are both PCI and AGP versions. You don't need to choose this to > + run the Radeon in plain VGA mode. > + > + If M is selected, the module will be called radeon. > + > config DRM_RADEON_USERPTR > bool "Always enable userptr support" > depends on DRM_RADEON -- ~Randy
[PATCH] drm: omapdrm: Improve check for contiguous buffers
While a scatter-gather table having only 1 entry does imply it is
contiguous, it is a logic error to assume the inverse. Tables can have
more than 1 entry and still be contiguous. Use a proper check here.
Signed-off-by: Andrew Davis
---
drivers/gpu/drm/omapdrm/omap_gem.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
b/drivers/gpu/drm/omapdrm/omap_gem.c
index cf571796fd26..52c00b795459 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -48,7 +48,7 @@ struct omap_gem_object {
* OMAP_BO_MEM_DMA_API flag set)
*
* - buffers imported from dmabuf (with the OMAP_BO_MEM_DMABUF flag set)
-* if they are physically contiguous (when sgt->orig_nents == 1)
+* if they are physically contiguous
*
* - buffers mapped through the TILER when pin_cnt is not zero, in which
* case the DMA address points to the TILER aperture
@@ -148,12 +148,18 @@ u64 omap_gem_mmap_offset(struct drm_gem_object *obj)
return drm_vma_node_offset_addr(&obj->vma_node);
}
+static bool omap_gem_sgt_is_contiguous(struct sg_table *sgt, size)
+{
+ return !(drm_prime_get_contiguous_size(sgt) < size);
+}
+
static bool omap_gem_is_contiguous(struct omap_gem_object *omap_obj)
{
if (omap_obj->flags & OMAP_BO_MEM_DMA_API)
return true;
- if ((omap_obj->flags & OMAP_BO_MEM_DMABUF) && omap_obj->sgt->nents == 1)
+ if ((omap_obj->flags & OMAP_BO_MEM_DMABUF) &&
+ omap_gem_sgt_is_contiguous(omap_obj->sgt, omap_obj->base->size))
return true;
return false;
@@ -1398,7 +1404,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct
drm_device *dev, size_t size,
union omap_gem_size gsize;
/* Without a DMM only physically contiguous buffers can be supported. */
- if (sgt->orig_nents != 1 && !priv->has_dmm)
+ if (!omap_gem_sgt_is_contiguous(sgt, size) && !priv->has_dmm)
return ERR_PTR(-EINVAL);
gsize.bytes = PAGE_ALIGN(size);
@@ -1412,7 +1418,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct
drm_device *dev, size_t size,
omap_obj->sgt = sgt;
- if (sgt->orig_nents == 1) {
+ if (omap_gem_sgt_is_contiguous(sgt, size)) {
omap_obj->dma_addr = sg_dma_address(sgt->sgl);
} else {
/* Create pages list from sgt */
--
2.36.1
[PATCH] drm: Move radeon and amdgpu Kconfig options into their directories
Most Kconfig options to enable a driver are in the Kconfig file inside the relevant directory, move these two to the same. Signed-off-by: Andrew Davis --- drivers/gpu/drm/Kconfig| 42 -- drivers/gpu/drm/amd/amdgpu/Kconfig | 22 drivers/gpu/drm/radeon/Kconfig | 22 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 6c2256e8474b..24fa9ccd92a4 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -234,50 +234,8 @@ source "drivers/gpu/drm/i2c/Kconfig" source "drivers/gpu/drm/arm/Kconfig" -config DRM_RADEON - tristate "ATI Radeon" - depends on DRM && PCI && MMU - depends on AGP || !AGP - select FW_LOADER - select DRM_DISPLAY_DP_HELPER - select DRM_DISPLAY_HELPER -select DRM_KMS_HELPER -select DRM_TTM - select DRM_TTM_HELPER - select POWER_SUPPLY - select HWMON - select BACKLIGHT_CLASS_DEVICE - select INTERVAL_TREE - help - Choose this option if you have an ATI Radeon graphics card. There - are both PCI and AGP versions. You don't need to choose this to - run the Radeon in plain VGA mode. - - If M is selected, the module will be called radeon. - source "drivers/gpu/drm/radeon/Kconfig" -config DRM_AMDGPU - tristate "AMD GPU" - depends on DRM && PCI && MMU - select FW_LOADER - select DRM_DISPLAY_DP_HELPER - select DRM_DISPLAY_HDMI_HELPER - select DRM_DISPLAY_HELPER - select DRM_KMS_HELPER - select DRM_SCHED - select DRM_TTM - select DRM_TTM_HELPER - select POWER_SUPPLY - select HWMON - select BACKLIGHT_CLASS_DEVICE - select INTERVAL_TREE - select DRM_BUDDY - help - Choose this option if you have a recent AMD Radeon graphics card. - - If M is selected, the module will be called amdgpu. - source "drivers/gpu/drm/amd/amdgpu/Kconfig" source "drivers/gpu/drm/nouveau/Kconfig" diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index d55275de..36b1206124cf 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -1,4 +1,26 @@ # SPDX-License-Identifier: MIT + +config DRM_AMDGPU + tristate "AMD GPU" + depends on DRM && PCI && MMU + select FW_LOADER + select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HDMI_HELPER + select DRM_DISPLAY_HELPER + select DRM_KMS_HELPER + select DRM_SCHED + select DRM_TTM + select DRM_TTM_HELPER + select POWER_SUPPLY + select HWMON + select BACKLIGHT_CLASS_DEVICE + select INTERVAL_TREE + select DRM_BUDDY + help + Choose this option if you have a recent AMD Radeon graphics card. + + If M is selected, the module will be called amdgpu. + config DRM_AMDGPU_SI bool "Enable amdgpu support for SI parts" depends on DRM_AMDGPU diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig index 52819e7f1fca..3248d12c562d 100644 --- a/drivers/gpu/drm/radeon/Kconfig +++ b/drivers/gpu/drm/radeon/Kconfig @@ -1,4 +1,26 @@ # SPDX-License-Identifier: MIT + +config DRM_RADEON + tristate "ATI Radeon" + depends on DRM && PCI && MMU + depends on AGP || !AGP + select FW_LOADER + select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_HELPER +select DRM_KMS_HELPER +select DRM_TTM + select DRM_TTM_HELPER + select POWER_SUPPLY + select HWMON + select BACKLIGHT_CLASS_DEVICE + select INTERVAL_TREE + help + Choose this option if you have an ATI Radeon graphics card. There + are both PCI and AGP versions. You don't need to choose this to + run the Radeon in plain VGA mode. + + If M is selected, the module will be called radeon. + config DRM_RADEON_USERPTR bool "Always enable userptr support" depends on DRM_RADEON -- 2.36.1
[PATCH] dma-buf: cma_heap: Check for device max segment size when attaching
Although there is usually not such a limitation (and when there is it is
often only because the driver forgot to change the super small default),
it is still correct here to break scatterlist element into chunks of
dma_max_mapping_size().
This might cause some issues for users with misbehaving drivers. If
bisecting has landed you on this commit, make sure your drivers both set
dma_set_max_seg_size() and are checking for contiguousness correctly.
Signed-off-by: Andrew Davis
---
drivers/dma-buf/heaps/cma_heap.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index 28fb04eccdd0..cacc84cb5ece 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -58,10 +58,11 @@ static int cma_heap_attach(struct dma_buf *dmabuf,
if (!a)
return -ENOMEM;
- ret = sg_alloc_table_from_pages(&a->table, buffer->pages,
- buffer->pagecount, 0,
- buffer->pagecount << PAGE_SHIFT,
- GFP_KERNEL);
+ size_t max_segment = dma_get_max_seg_size(attachment->dev);
+ ret = sg_alloc_table_from_pages_segment(&a->table, buffer->pages,
+ buffer->pagecount, 0,
+ buffer->pagecount << PAGE_SHIFT,
+ max_segment, GFP_KERNEL);
if (ret) {
kfree(a);
return ret;
--
2.36.1
Re: [PATCH v2 6/9] arm64: dts: qcom: sc7280: drop unused clocks from eDP node
Quoting Dmitry Baryshkov (2022-07-10 01:41:30) > The eDP node includes two clocks which are used by the eDP PHY rather > than eDP controller itself. Drop these clocks to remove extra difference > between eDP and DP controllers. > > Suggested-by: Stephen Boyd > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
[PATCH] drm/panfrost: Update io-pgtable API
Convert to io-pgtable's bulk {map,unmap}_pages() APIs, to help the old
single-page interfaces eventually go away. Unmapping heap BOs still
wants to be done a page at a time, but everything else can get the full
benefit of the more efficient interface.
Signed-off-by: Robin Murphy
---
drivers/gpu/drm/panfrost/panfrost_mmu.c | 40 +++--
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index b285a8001b1d..e246d914e7f6 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -248,11 +248,15 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
mmu_write(pfdev, MMU_INT_MASK, ~0);
}
-static size_t get_pgsize(u64 addr, size_t size)
+static size_t get_pgsize(u64 addr, size_t size, size_t *count)
{
- if (addr & (SZ_2M - 1) || size < SZ_2M)
- return SZ_4K;
+ size_t blk_offset = -addr % SZ_2M;
+ if (blk_offset || size < SZ_2M) {
+ *count = min_not_zero(blk_offset, size) / SZ_4K;
+ return SZ_4K;
+ }
+ *count = size / SZ_2M;
return SZ_2M;
}
@@ -287,12 +291,16 @@ static int mmu_map_sg(struct panfrost_device *pfdev,
struct panfrost_mmu *mmu,
dev_dbg(pfdev->dev, "map: as=%d, iova=%llx, paddr=%lx,
len=%zx", mmu->as, iova, paddr, len);
while (len) {
- size_t pgsize = get_pgsize(iova | paddr, len);
+ size_t pgcount, mapped = 0;
+ size_t pgsize = get_pgsize(iova | paddr, len, &pgcount);
- ops->map(ops, iova, paddr, pgsize, prot, GFP_KERNEL);
- iova += pgsize;
- paddr += pgsize;
- len -= pgsize;
+ ops->map_pages(ops, iova, paddr, pgsize, pgcount, prot,
+ GFP_KERNEL, &mapped);
+ /* Don't get stuck if things have gone wrong */
+ mapped = max(mapped, pgsize);
+ iova += mapped;
+ paddr += mapped;
+ len -= mapped;
}
}
@@ -344,15 +352,17 @@ void panfrost_mmu_unmap(struct panfrost_gem_mapping
*mapping)
mapping->mmu->as, iova, len);
while (unmapped_len < len) {
- size_t unmapped_page;
- size_t pgsize = get_pgsize(iova, len - unmapped_len);
+ size_t unmapped_page, pgcount;
+ size_t pgsize = get_pgsize(iova, len - unmapped_len, &pgcount);
- if (ops->iova_to_phys(ops, iova)) {
- unmapped_page = ops->unmap(ops, iova, pgsize, NULL);
- WARN_ON(unmapped_page != pgsize);
+ if (bo->is_heap)
+ pgcount = 1;
+ if (!bo->is_heap || ops->iova_to_phys(ops, iova)) {
+ unmapped_page = ops->unmap_pages(ops, iova, pgsize,
pgcount, NULL);
+ WARN_ON(unmapped_page != pgsize * pgcount);
}
- iova += pgsize;
- unmapped_len += pgsize;
+ iova += pgsize * pgcount;
+ unmapped_len += pgsize * pgcount;
}
panfrost_mmu_flush_range(pfdev, mapping->mmu,
--
2.36.1.dirty
Re: [PATCH] drm/nouveau/hwmon: use simplified HWMON_CHANNEL_INFO macro
Reviewed-by: Lyude Paul
Thanks! I will push this upstream in a moment
On Mon, 2022-08-15 at 13:40 +0300, Beniamin Sandu wrote:
> This makes the code look cleaner and easier to read.
>
> Signed-off-by: Beniamin Sandu
> ---
> drivers/gpu/drm/nouveau/nouveau_hwmon.c | 85 +
> 1 file changed, 17 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> index 1c3104d20571..a7db7c31064b 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> @@ -211,75 +211,24 @@ static const struct attribute_group
> temp1_auto_point_sensor_group = {
>
> #define N_ATTR_GROUPS 3
>
> -static const u32 nouveau_config_chip[] = {
> - HWMON_C_UPDATE_INTERVAL,
> - 0
> -};
> -
> -static const u32 nouveau_config_in[] = {
> - HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX | HWMON_I_LABEL,
> - 0
> -};
> -
> -static const u32 nouveau_config_temp[] = {
> - HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST |
> - HWMON_T_CRIT | HWMON_T_CRIT_HYST | HWMON_T_EMERGENCY |
> - HWMON_T_EMERGENCY_HYST,
> - 0
> -};
> -
> -static const u32 nouveau_config_fan[] = {
> - HWMON_F_INPUT,
> - 0
> -};
> -
> -static const u32 nouveau_config_pwm[] = {
> - HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
> - 0
> -};
> -
> -static const u32 nouveau_config_power[] = {
> - HWMON_P_INPUT | HWMON_P_CAP_MAX | HWMON_P_CRIT,
> - 0
> -};
> -
> -static const struct hwmon_channel_info nouveau_chip = {
> - .type = hwmon_chip,
> - .config = nouveau_config_chip,
> -};
> -
> -static const struct hwmon_channel_info nouveau_temp = {
> - .type = hwmon_temp,
> - .config = nouveau_config_temp,
> -};
> -
> -static const struct hwmon_channel_info nouveau_fan = {
> - .type = hwmon_fan,
> - .config = nouveau_config_fan,
> -};
> -
> -static const struct hwmon_channel_info nouveau_in = {
> - .type = hwmon_in,
> - .config = nouveau_config_in,
> -};
> -
> -static const struct hwmon_channel_info nouveau_pwm = {
> - .type = hwmon_pwm,
> - .config = nouveau_config_pwm,
> -};
> -
> -static const struct hwmon_channel_info nouveau_power = {
> - .type = hwmon_power,
> - .config = nouveau_config_power,
> -};
> -
> static const struct hwmon_channel_info *nouveau_info[] = {
> - &nouveau_chip,
> - &nouveau_temp,
> - &nouveau_fan,
> - &nouveau_in,
> - &nouveau_pwm,
> - &nouveau_power,
> + HWMON_CHANNEL_INFO(chip,
> +HWMON_C_UPDATE_INTERVAL),
> + HWMON_CHANNEL_INFO(temp,
> +HWMON_T_INPUT |
> +HWMON_T_MAX | HWMON_T_MAX_HYST |
> +HWMON_T_CRIT | HWMON_T_CRIT_HYST |
> +HWMON_T_EMERGENCY | HWMON_T_EMERGENCY_HYST),
> + HWMON_CHANNEL_INFO(fan,
> +HWMON_F_INPUT),
> + HWMON_CHANNEL_INFO(in,
> +HWMON_I_INPUT |
> +HWMON_I_MIN | HWMON_I_MAX |
> +HWMON_I_LABEL),
> + HWMON_CHANNEL_INFO(pwm,
> +HWMON_PWM_INPUT | HWMON_PWM_ENABLE),
> + HWMON_CHANNEL_INFO(power,
> +HWMON_P_INPUT | HWMON_P_CAP_MAX | HWMON_P_CRIT),
> NULL
> };
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [PATCH 0/4] Allow MMIO regions to be exported through dma-buf
On Thu, 18 Aug 2022 09:05:24 -0300 Jason Gunthorpe wrote: > On Wed, Aug 17, 2022 at 01:11:38PM -0300, Jason Gunthorpe wrote: > > dma-buf has become a way to safely acquire a handle to non-struct page > > memory that can still have lifetime controlled by the exporter. Notably > > RDMA can now import dma-buf FDs and build them into MRs which allows for > > PCI P2P operations. Extend this to allow vfio-pci to export MMIO memory > > from PCI device BARs. > > > > This series supports a use case for SPDK where a NVMe device will be owned > > by SPDK through VFIO but interacting with a RDMA device. The RDMA device > > may directly access the NVMe CMB or directly manipulate the NVMe device's > > doorbell using PCI P2P. > > > > However, as a general mechanism, it can support many other scenarios with > > VFIO. I imagine this dmabuf approach to be usable by iommufd as well for > > generic and safe P2P mappings. > > > > This series goes after the "Break up ioctl dispatch functions to one > > function per ioctl" series. > > > > This is on github: https://github.com/jgunthorpe/linux/commits/vfio_dma_buf > > > > Jason Gunthorpe (4): > > dma-buf: Add dma_buf_try_get() > > vfio: Add vfio_device_get() > > vfio_pci: Do not open code pci_try_reset_function() > > vfio/pci: Allow MMIO regions to be exported through dma-buf > > > > drivers/vfio/pci/Makefile | 1 + > > drivers/vfio/pci/vfio_pci_config.c | 22 ++- > > drivers/vfio/pci/vfio_pci_core.c| 33 +++- > > drivers/vfio/pci/vfio_pci_dma_buf.c | 265 > > I forget about this.. > > Alex, do you want to start doing as Linus discused and I will rename > this new file to "dma_buf.c" ? > > Or keep this directory as having the vfio_pci_* prefix for > consistency? I have a hard time generating a strong opinion over file name redundancy relative to directory structure. By my count, over 17% of files in drivers/ have some file name redundancy to their parent directory structure (up to two levels). I see we already have two $FOO_dma_buf.c files in the tree, virtio and amdgpu among these. In the virtio case this is somewhat justified, to me at least, as the virtio_dma_buf.h file exists in a shared include namespace. However, this justification only accounts for about 1% of such files, for many others the redundancy exists in the include path as well. I guess if we don't have a reason other than naming consistency and accept an end goal to incrementally remove file name vs directory structure redundancy where it makes sense, sure, name it dma_buf.c. Ugh. Thanks, Alex
Re: [PATCH] nouveau: explicitly wait on the fence in nouveau_bo_move_m2mf
Reviewed-by: Lyude Paul
On Fri, 2022-08-19 at 22:09 +0200, Karol Herbst wrote:
> It is a bit unlcear to us why that's helping, but it does and unbreaks
> suspend/resume on a lot of GPUs without any known drawbacks.
>
> Cc: sta...@vger.kernel.org # v5.15+
> Closes: https://gitlab.freedesktop.org/drm/nouveau/-/issues/156
> Signed-off-by: Karol Herbst
> ---
> drivers/gpu/drm/nouveau/nouveau_bo.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 35bb0bb3fe61..126b3c6e12f9 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -822,6 +822,15 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int
> evict,
> if (ret == 0) {
> ret = nouveau_fence_new(chan, false, &fence);
> if (ret == 0) {
> + /* TODO: figure out a better solution here
> + *
> + * wait on the fence here explicitly as going
> through
> + * ttm_bo_move_accel_cleanup somehow doesn't
> seem to do it.
> + *
> + * Without this the operation can timeout and
> we'll fallback to a
> + * software copy, which might take several
> minutes to finish.
> + */
> + nouveau_fence_wait(fence, false, false);
> ret = ttm_bo_move_accel_cleanup(bo,
> &fence->base,
> evict, false,
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [PATCH] drm/msm: Add fault-injection support
On 07/08/2022 20:28, Rob Clark wrote: From: Rob Clark Intended as a way to trigger error paths in mesa. Signed-off-by: Rob Clark Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/msm_debugfs.c | 8 drivers/gpu/drm/msm/msm_drv.c | 15 +++ drivers/gpu/drm/msm/msm_drv.h | 7 +++ 3 files changed, 30 insertions(+) -- With best wishes Dmitry
Re: [PATCH 6/7] drm/msm/dp: Don't enable HPD interrupts for edp
On 10/08/2022 06:50, Bjorn Andersson wrote: Most instances where HPD interrupts are masked and unmasked are guareded by the presence of an EDP panel being connected, but not all. Extend this to cover the last few places, as HPD interrupt handling is not used for the EDP case. Signed-off-by: Bjorn Andersson Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dp/dp_display.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) -- With best wishes Dmitry
Re: [PATCH 5/7] drm/msm/dp: Implement hpd_notify()
On 10/08/2022 06:50, Bjorn Andersson wrote:
The DisplayPort controller's hot-plug mechanism is based on pinmuxing a
physical signal no a GPIO pin into the controller. This is not always
possible, either because there aren't dedicated GPIOs available or
because the hot-plug signal is a virtual notification, in cases such as
USB Type-C.
For these cases, by implementing the hpd_notify() callback for the
DisplayPort controller's drm_bridge, a downstream drm_bridge
(next_bridge) can be used to track and signal the connection status
changes.
This makes it possible to use downstream drm_bridges such as
display-connector or any virtual mechanism, as long as they are
implemented as a drm_bridge.
Signed-off-by: Bjorn Andersson
---
drivers/gpu/drm/msm/dp/dp_display.c | 23 +++
drivers/gpu/drm/msm/dp/dp_display.h | 1 +
drivers/gpu/drm/msm/dp/dp_drm.c | 3 +++
drivers/gpu/drm/msm/dp/dp_drm.h | 2 ++
4 files changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 699f28f2251e..568295381246 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1303,6 +1303,7 @@ static int dp_display_probe(struct platform_device *pdev)
if (!desc)
return -EINVAL;
+ dp->dp_display.dev = &pdev->dev;
dp->pdev = pdev;
dp->name = "drm_dp";
dp->id = desc->id;
@@ -1765,3 +1766,25 @@ void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
dp_display->dp_mode.h_active_low =
!!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC);
}
+
+void dp_bridge_hpd_notify(struct drm_bridge *bridge,
+ enum drm_connector_status status)
+{
+ struct msm_dp_bridge *dp_bridge = to_dp_bridge(bridge);
+ struct msm_dp *dp_display = dp_bridge->dp_display;
+ struct dp_display_private *dp = container_of(dp_display, struct
dp_display_private, dp_display);
+
+ /* Without next_bridge interrupts are handled by the DP core directly */
+ if (!dp_display->next_bridge)
+ return;
+
+ if (!dp->core_initialized) {
+ drm_dbg_dp(dp->drm_dev, "not initialized\n");
+ return;
+ }
+
+ if (!dp_display->is_connected && status == connector_status_connected)
+ dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+ else if (dp_display->is_connected && status ==
connector_status_disconnected)
+ dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+}
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index dcedf021f7fe..d7bc537ead31 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -11,6 +11,7 @@
#include "disp/msm_disp_snapshot.h"
struct msm_dp {
+ struct device *dev;
struct drm_device *drm_dev;
struct device *codec_dev;
struct drm_bridge *bridge;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 6df25f7662e7..875b23910bef 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -68,6 +68,7 @@ static const struct drm_bridge_funcs dp_bridge_ops = {
.mode_valid = dp_bridge_mode_valid,
.get_modes= dp_bridge_get_modes,
.detect = dp_bridge_detect,
+ .hpd_notify = dp_bridge_hpd_notify,
};
struct drm_bridge *dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
@@ -138,6 +139,8 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp
*dp_display, struct dr
if (IS_ERR(connector))
return connector;
+ connector->fwnode = fwnode_handle_get(dev_fwnode(dp_display->dev));
Not used anymore, isn't it? Then dp_display->dev also seems unused.
+
drm_connector_attach_encoder(connector, encoder);
return connector;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h
index 82035dbb0578..79e6b2cf2d25 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.h
+++ b/drivers/gpu/drm/msm/dp/dp_drm.h
@@ -32,5 +32,7 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge
*bridge,
void dp_bridge_mode_set(struct drm_bridge *drm_bridge,
const struct drm_display_mode *mode,
const struct drm_display_mode *adjusted_mode);
+void dp_bridge_hpd_notify(struct drm_bridge *bridge,
+ enum drm_connector_status status);
#endif /* _DP_DRM_H_ */
--
With best wishes
Dmitry
Re: [PATCH 4/7] drm/msm/dp: Add SDM845 DisplayPort instance
On 10/08/2022 06:50, Bjorn Andersson wrote: The Qualcomm SDM845 platform has a single DisplayPort controller, with the same design as SC7180, so add support for this by reusing the SC7180 definition. Signed-off-by: Bjorn Andersson Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dp/dp_display.c | 1 + 1 file changed, 1 insertion(+) -- With best wishes Dmitry
Re: [RESEND v6 6/9] drm: vkms: Refactor the plane composer to accept new formats
On 08/22, Igor Matheus Andrade Torrente wrote: > On 8/22/22 16:01, Melissa Wen wrote: > > On 08/22, Igor Matheus Andrade Torrente wrote: > > > Hi Melissa, > > > > > > On 8/20/22 07:51, Melissa Wen wrote: > > > > On 08/19, Igor Torrente wrote: > > > > > Currently the blend function only accepts XRGB_ and ARGB_ > > > > > as a color input. > > > > > > > > > > This patch refactors all the functions related to the plane > > > > > composition > > > > > to overcome this limitation. > > > > > > > > > > The pixels blend is done using the new internal format. And new > > > > > handlers > > > > > are being added to convert a specific format to/from this internal > > > > > format. > > > > > > > > > > So the blend operation depends on these handlers to convert to this > > > > > common > > > > > format. The blended result, if necessary, is converted to the > > > > > writeback > > > > > buffer format. > > > > > > > > > > This patch introduces three major differences to the blend function. > > > > > 1 - All the planes are blended at once. > > > > > 2 - The blend calculus is done as per line instead of per pixel. > > > > > 3 - It is responsible to calculates the CRC and writing the writeback > > > > > buffer(if necessary). > > > > > > > > > > These changes allow us to allocate way less memory in the intermediate > > > > > buffer to compute these operations. Because now we don't need to > > > > > have the entire intermediate image lines at once, just one line is > > > > > enough. > > > > > > > > > > | Memory consumption (output dimensions) | > > > > > |:--:| > > > > > | Current | This patch| > > > > > |:--:|:-:| > > > > > | Width * Heigth | 2 * Width | > > > > > > > > > > Beyond memory, we also have a minor performance benefit from all > > > > > these changes. Results running the IGT[1] test > > > > > `igt@kms_cursor_crc@pipe-a-cursor-512x512-onscreen` ten times: > > > > > > > > > > | Frametime | > > > > > |:--:| > > > > > | Implementation | Current | This commit | > > > > > |:---:|:-:|::| > > > > > | frametime range | 9~22 ms |5~17 ms | > > > > > | Average | 11.4 ms |7.8 ms| > > > > > > > > > > [1] IGT commit id: bc3f6833a12221a46659535dac06ebb312490eb4 > > > > > > > > > > V2: Improves the performance drastically, by performing the operations > > > > > per-line and not per-pixel(Pekka Paalanen). > > > > > Minor improvements(Pekka Paalanen). > > > > > V3: Changes the code to blend the planes all at once. This improves > > > > > performance, memory consumption, and removes much of the > > > > > weirdness > > > > > of the V2(Pekka Paalanen and me). > > > > > Minor improvements(Pekka Paalanen and me). > > > > > V4: Rebase the code and adapt it to the new NUM_OVERLAY_PLANES > > > > > constant. > > > > > V5: Minor checkpatch fixes and the removal of TO-DO item(Melissa Wen). > > > > > Several security/robustness improvents(Pekka Paalanen). > > > > > Removes check_planes_x_bounds function and allows partial > > > > > partly off-screen(Pekka Paalanen). > > > > > V6: Fix a mismatch of some variable sizes (Pekka Paalanen). > > > > > Several minor improvements (Pekka Paalanen). > > > > > > > > > > Reported-by: kernel test robot > > > > > Signed-off-by: Igor Torrente > > > > > --- > > > > >Documentation/gpu/vkms.rst| 4 - > > > > >drivers/gpu/drm/vkms/Makefile | 1 + > > > > >drivers/gpu/drm/vkms/vkms_composer.c | 320 > > > > > -- > > > > >drivers/gpu/drm/vkms/vkms_formats.c | 155 + > > > > >drivers/gpu/drm/vkms/vkms_formats.h | 12 + > > > > >drivers/gpu/drm/vkms/vkms_plane.c | 3 + > > > > >drivers/gpu/drm/vkms/vkms_writeback.c | 3 + > > > > >7 files changed, 317 insertions(+), 181 deletions(-) > > > > >create mode 100644 drivers/gpu/drm/vkms/vkms_formats.c > > > > >create mode 100644 drivers/gpu/drm/vkms/vkms_formats.h > > > > > > > > > > diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst > > > > > index 973e2d43108b..a49e4ae92653 100644 > > > > > --- a/Documentation/gpu/vkms.rst > > > > > +++ b/Documentation/gpu/vkms.rst > > > > > @@ -118,10 +118,6 @@ Add Plane Features > > > > >There's lots of plane features we could add support for: > > > > > -- Clearing primary plane: clear primary plane before plane > > > > > composition (at the > > > > > - start) for correctness of pixel blend ops. It also guarantees > > > > > alpha channel > > > > > - is cleared in the target buffer for stable crc. [Good to get > > > > > started] > > > > > - > > > > >- ARGB format on primary plane: blend the primary plane into > > > > > background with > > > > > translucent alpha. > > > > > diff --git
[PATCH v3 9/9] dt-bindings: display/msm/dpu-common: add opp-table property
The display controller node can contain the opp-table describing its frequencies and OPP levels. Allow specifying the opp-table in the DPU devices. Signed-off-by: Dmitry Baryshkov --- Documentation/devicetree/bindings/display/msm/dpu-common.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/dpu-common.yaml b/Documentation/devicetree/bindings/display/msm/dpu-common.yaml index 14eda883e149..42e1616a5670 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-common.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-common.yaml @@ -22,6 +22,9 @@ properties: operating-points-v2: true + opp-table: +type: object + ports: $ref: /schemas/graph.yaml#/properties/ports description: | -- 2.35.1
[PATCH v3 3/9] dt-bindings: display/msm: move qcom, sc7180-mdss schema to mdss.yaml
Move schema for qcom,sc7180-mdss from dpu-sc7180.yaml to mdss.yaml so that the dpu file describes only the DPU schema. Reviewed-by: Rob Herring Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dpu-sc7180.yaml | 149 +- .../devicetree/bindings/display/msm/mdss.yaml | 45 +- 2 files changed, 80 insertions(+), 114 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml index d3c3e4b07897..9d4ec0b60c25 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml @@ -10,151 +10,78 @@ maintainers: - Krishna Manikandan description: | - Device tree bindings for MSM Mobile Display Subsystem(MDSS) that encapsulates - sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree - bindings of MDSS and DPU are mentioned for SC7180 target. + Device tree bindings for the DPU display controller for SC7180 target. properties: compatible: items: - - const: qcom,sc7180-mdss + - const: qcom,sc7180-dpu reg: -maxItems: 1 +items: + - description: Address offset and size for mdp register set + - description: Address offset and size for vbif register set reg-names: -const: mdss - - power-domains: -maxItems: 1 +items: + - const: mdp + - const: vbif clocks: items: - - description: Display AHB clock from gcc - - description: Display AHB clock from dispcc + - description: Display hf axi clock + - description: Display ahb clock + - description: Display rotator clock + - description: Display lut clock - description: Display core clock + - description: Display vsync clock clock-names: items: + - const: bus - const: iface - - const: ahb + - const: rot + - const: lut - const: core + - const: vsync interrupts: maxItems: 1 - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": -const: 1 - - iommus: -items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - ranges: true - - interconnects: -items: - - description: Interconnect path specifying the port ids for data bus - - interconnect-names: -const: mdp0-mem + power-domains: +maxItems: 1 - resets: -items: - - description: MDSS_CORE reset + operating-points-v2: true -patternProperties: - "^display-controller@[0-9a-f]+$": -type: object -description: Node containing the properties of DPU. + ports: +$ref: /schemas/graph.yaml#/properties/ports +description: | + Contains the list of output ports from DPU device. These ports + connect to interfaces that are external to the DPU hardware, + such as DSI, DP etc. Each output port contains an endpoint that + describes how it is connected to an external interface. properties: - compatible: -items: - - const: qcom,sc7180-dpu - - reg: -items: - - description: Address offset and size for mdp register set - - description: Address offset and size for vbif register set - - reg-names: -items: - - const: mdp - - const: vbif - - clocks: -items: - - description: Display hf axi clock - - description: Display ahb clock - - description: Display rotator clock - - description: Display lut clock - - description: Display core clock - - description: Display vsync clock - - clock-names: -items: - - const: bus - - const: iface - - const: rot - - const: lut - - const: core - - const: vsync - - interrupts: -maxItems: 1 - - power-domains: -maxItems: 1 - - operating-points-v2: true - - ports: -$ref: /schemas/graph.yaml#/properties/ports -description: | - Contains the list of output ports from DPU device. These ports - connect to interfaces that are external to the DPU hardware, - such as DSI, DP etc. Each output port contains an endpoint that - describes how it is connected to an external interface. - -properties: - port@0: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF1 (DSI1) - - port@2: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF0 (DP) - -required: - - port@0 + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF1 (DSI1) + + port@2: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF0 (DP) required: - - compatible - - reg - - reg-names -
[PATCH v3 4/9] dt-bindings: display/msm: move qcom, sc7280-mdss schema to mdss.yaml
Move schema for qcom,sc7280-mdss from dpu-sc7280.yaml to mdss.yaml so that the dpu file describes only the DPU schema. Reviewed-by: Rob Herring Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dpu-sc7280.yaml | 148 +- .../devicetree/bindings/display/msm/mdss.yaml | 19 +++ 2 files changed, 57 insertions(+), 110 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml index f427eec3d3a4..349a454099ad 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml @@ -10,149 +10,77 @@ maintainers: - Krishna Manikandan description: | - Device tree bindings for MSM Mobile Display Subsystem (MDSS) that encapsulates - sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree - bindings of MDSS and DPU are mentioned for SC7280. + Device tree bindings for the DPU display controller for SC7280 target. properties: compatible: -const: qcom,sc7280-mdss +const: qcom,sc7280-dpu reg: -maxItems: 1 +items: + - description: Address offset and size for mdp register set + - description: Address offset and size for vbif register set reg-names: -const: mdss - - power-domains: -maxItems: 1 +items: + - const: mdp + - const: vbif clocks: items: - - description: Display AHB clock from gcc - - description: Display AHB clock from dispcc + - description: Display hf axi clock + - description: Display sf axi clock + - description: Display ahb clock + - description: Display lut clock - description: Display core clock + - description: Display vsync clock clock-names: items: + - const: bus + - const: nrt_bus - const: iface - - const: ahb + - const: lut - const: core + - const: vsync interrupts: maxItems: 1 - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": -const: 1 - - iommus: -items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - ranges: true - - interconnects: -items: - - description: Interconnect path specifying the port ids for data bus - - interconnect-names: -const: mdp0-mem + power-domains: +maxItems: 1 - resets: -items: - - description: MDSS_CORE reset + operating-points-v2: true -patternProperties: - "^display-controller@[0-9a-f]+$": -type: object -description: Node containing the properties of DPU. + ports: +$ref: /schemas/graph.yaml#/properties/ports +description: | + Contains the list of output ports from DPU device. These ports + connect to interfaces that are external to the DPU hardware, + such as DSI, DP etc. Each output port contains an endpoint that + describes how it is connected to an external interface. properties: - compatible: -const: qcom,sc7280-dpu - - reg: -items: - - description: Address offset and size for mdp register set - - description: Address offset and size for vbif register set - - reg-names: -items: - - const: mdp - - const: vbif - - clocks: -items: - - description: Display hf axi clock - - description: Display sf axi clock - - description: Display ahb clock - - description: Display lut clock - - description: Display core clock - - description: Display vsync clock - - clock-names: -items: - - const: bus - - const: nrt_bus - - const: iface - - const: lut - - const: core - - const: vsync - - interrupts: -maxItems: 1 - - power-domains: -maxItems: 1 - - operating-points-v2: true - - ports: -$ref: /schemas/graph.yaml#/properties/ports -description: | - Contains the list of output ports from DPU device. These ports - connect to interfaces that are external to the DPU hardware, - such as DSI, DP etc. Each output port contains an endpoint that - describes how it is connected to an external interface. - -properties: - port@0: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF1 (DSI) - - port@1: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF5 (EDP) - -required: - - port@0 + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF1 (DSI) + + port@1: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF5 (EDP) required: - - compatible - - reg - - reg-names - - clocks - - interrupts - - pow
[PATCH v3 6/9] dt-bindings: display/msm: move qcom, msm8998-mdss schema to mdss.yaml
Move schema for qcom,msm8998-mdss from dpu-msm8998.yaml to mdss.yaml so that the dpu file describes only the DPU schema. Reviewed-by: Rob Herring Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dpu-msm8998.yaml | 142 +- .../devicetree/bindings/display/msm/mdss.yaml | 24 +++ 2 files changed, 64 insertions(+), 102 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml index 2df64afb76e6..5caf46a1dd88 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml @@ -10,142 +10,80 @@ maintainers: - AngeloGioacchino Del Regno description: | - Device tree bindings for MSM Mobile Display Subsystem(MDSS) that encapsulates - sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree - bindings of MDSS and DPU are mentioned for MSM8998 target. + Device tree bindings for the DPU display controller for MSM8998 target. properties: compatible: items: - - const: qcom,msm8998-mdss + - const: qcom,msm8998-dpu reg: -maxItems: 1 +items: + - description: Address offset and size for mdp register set + - description: Address offset and size for regdma register set + - description: Address offset and size for vbif register set + - description: Address offset and size for non-realtime vbif register set reg-names: -const: mdss - - power-domains: -maxItems: 1 +items: + - const: mdp + - const: regdma + - const: vbif + - const: vbif_nrt clocks: items: - - description: Display AHB clock - - description: Display AXI clock + - description: Display ahb clock + - description: Display axi clock + - description: Display mem-noc clock - description: Display core clock + - description: Display vsync clock clock-names: items: - const: iface - const: bus + - const: mnoc - const: core + - const: vsync interrupts: maxItems: 1 - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": -const: 1 - - iommus: -items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - ranges: true + power-domains: +maxItems: 1 -patternProperties: - "^display-controller@[0-9a-f]+$": -type: object -description: Node containing the properties of DPU. + operating-points-v2: true + ports: +$ref: /schemas/graph.yaml#/properties/ports +description: | + Contains the list of output ports from DPU device. These ports + connect to interfaces that are external to the DPU hardware, + such as DSI, DP etc. Each output port contains an endpoint that + describes how it is connected to an external interface. properties: - compatible: -items: - - const: qcom,msm8998-dpu - - reg: -items: - - description: Address offset and size for mdp register set - - description: Address offset and size for regdma register set - - description: Address offset and size for vbif register set - - description: Address offset and size for non-realtime vbif register set - - reg-names: -items: - - const: mdp - - const: regdma - - const: vbif - - const: vbif_nrt - - clocks: -items: - - description: Display ahb clock - - description: Display axi clock - - description: Display mem-noc clock - - description: Display core clock - - description: Display vsync clock - - clock-names: -items: - - const: iface - - const: bus - - const: mnoc - - const: core - - const: vsync - - interrupts: -maxItems: 1 - - power-domains: -maxItems: 1 - - operating-points-v2: true - ports: -$ref: /schemas/graph.yaml#/properties/ports -description: | - Contains the list of output ports from DPU device. These ports - connect to interfaces that are external to the DPU hardware, - such as DSI, DP etc. Each output port contains an endpoint that - describes how it is connected to an external interface. - -properties: - port@0: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF1 (DSI1) - - port@1: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF2 (DSI2) - -required: - - port@0 - - port@1 + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF1 (DSI1) + + port@1: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF2 (DSI2) requir
[PATCH v3 1/9] dt-bindings: display/msm: split qcom,mdss bindings
Split Mobile Display SubSystem (MDSS) root node bindings to the separate yaml file. Changes to the existing (txt) schema: - Added optional "vbif_nrt_phys" region used by msm8996 - Made "bus" and "vsync" clocks optional (they are not used by some platforms) - Added (optional) "core" clock added recently to the mdss driver - Added optional resets property referencing MDSS reset - Defined child nodes pointing to corresponding reference schema. - Dropped the "lut" clock. It was added to the schema by mistake (it is a part of mdp4 schema, not the mdss). Signed-off-by: Dmitry Baryshkov --- .../devicetree/bindings/display/msm/mdp5.txt | 30 +--- .../devicetree/bindings/display/msm/mdss.yaml | 161 ++ 2 files changed, 162 insertions(+), 29 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/mdss.yaml diff --git a/Documentation/devicetree/bindings/display/msm/mdp5.txt b/Documentation/devicetree/bindings/display/msm/mdp5.txt index 43d11279c925..65d03c58dee6 100644 --- a/Documentation/devicetree/bindings/display/msm/mdp5.txt +++ b/Documentation/devicetree/bindings/display/msm/mdp5.txt @@ -2,37 +2,9 @@ Qualcomm adreno/snapdragon MDP5 display controller Description: -This is the bindings documentation for the Mobile Display Subsytem(MDSS) that -encapsulates sub-blocks like MDP5, DSI, HDMI, eDP etc, and the MDP5 display +This is the bindings documentation for the MDP5 display controller found in SoCs like MSM8974, APQ8084, MSM8916, MSM8994 and MSM8996. -MDSS: -Required properties: -- compatible: - * "qcom,mdss" - MDSS -- reg: Physical base address and length of the controller's registers. -- reg-names: The names of register regions. The following regions are required: - * "mdss_phys" - * "vbif_phys" -- interrupts: The interrupt signal from MDSS. -- interrupt-controller: identifies the node as an interrupt controller. -- #interrupt-cells: specifies the number of cells needed to encode an interrupt - source, should be 1. -- power-domains: a power domain consumer specifier according to - Documentation/devicetree/bindings/power/power_domain.txt -- clocks: device clocks. See ../clocks/clock-bindings.txt for details. -- clock-names: the following clocks are required. - * "iface" - * "bus" - * "vsync" -- #address-cells: number of address cells for the MDSS children. Should be 1. -- #size-cells: Should be 1. -- ranges: parent bus address space is the same as the child bus address space. - -Optional properties: -- clock-names: the following clocks are optional: - * "lut" - MDP5: Required properties: - compatible: diff --git a/Documentation/devicetree/bindings/display/msm/mdss.yaml b/Documentation/devicetree/bindings/display/msm/mdss.yaml new file mode 100644 index ..6a8ec8310f6f --- /dev/null +++ b/Documentation/devicetree/bindings/display/msm/mdss.yaml @@ -0,0 +1,161 @@ +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/mdss.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Mobile Display SubSystem (MDSS) + +maintainers: + - Dmitry Baryshkov + - Rob Clark + +description: + This is the bindings documentation for the Mobile Display Subsytem(MDSS) that + encapsulates sub-blocks like MDP5, DSI, HDMI, eDP, etc. + +properties: + compatible: +enum: + - qcom,mdss + + reg: +minItems: 2 +maxItems: 3 + + reg-names: +minItems: 2 +items: + - const: mdss_phys + - const: vbif_phys + - const: vbif_nrt_phys + + interrupts: +maxItems: 1 + + interrupt-controller: +true + + "#interrupt-cells": +const: 1 + + power-domains: +maxItems: 1 +description: | + The MDSS power domain provided by GCC + + clocks: +minItems: 1 +maxItems: 4 + + clock-names: +minItems: 1 +maxItems: 4 + + "#address-cells": +const: 1 + + "#size-cells": +const: 1 + + ranges: +true + + resets: +items: + - description: MDSS_CORE reset + +oneOf: + - properties: + clocks: +minItems: 3 +maxItems: 4 + + clock-names: +minItems: 3 +items: + - const: iface + - const: bus + - const: vsync + - const: core + - properties: + clocks: +minItems: 1 +maxItems: 2 + + clock-names: +minItems: 1 +items: + - const: iface + - const: core + +required: + - compatible + - reg + - reg-names + - interrupts + - interrupt-controller + - "#interrupt-cells" + - power-domains + - clocks + - clock-names + - "#address-cells" + - "#size-cells" + - ranges + +patternProperties: + "^mdp@[1-9a-f][0-9a-f]*$": +type: object +# TODO: add reference once the mdp5 is converted + + "^dsi@[1-9a-f][0-9a-f]*$": +$ref: dsi-controller-main.yaml# + + "^dsi-phy@[1-9a-f][0-9a-f]*$": +oneOf: + - $ref: dsi-phy-28nm.yaml# + - $ref: dsi-phy-20nm.yaml#
[PATCH v3 5/9] dt-bindings: display/msm: move qcom, qcm2290-mdss schema to mdss.yaml
Move schema for qcom,qcm2290-mdss from dpu-qcm2290.yaml to mdss.yaml so that the dpu file describes only the DPU schema. Reviewed-by: Rob Herring Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dpu-qcm2290.yaml | 140 +- .../devicetree/bindings/display/msm/mdss.yaml | 24 +++ 2 files changed, 57 insertions(+), 107 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml b/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml index 734d14de966d..8027319b1aad 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml @@ -10,146 +10,72 @@ maintainers: - Loic Poulain description: | - Device tree bindings for MSM Mobile Display Subsystem(MDSS) that encapsulates - sub-blocks like DPU display controller and DSI. Device tree bindings of MDSS - and DPU are mentioned for QCM2290 target. + Device tree bindings for the DPU display controller for QCM2290 target. properties: compatible: items: - - const: qcom,qcm2290-mdss + - const: qcom,qcm2290-dpu reg: -maxItems: 1 +items: + - description: Address offset and size for mdp register set + - description: Address offset and size for vbif register set reg-names: -const: mdss - - power-domains: -maxItems: 1 +items: + - const: mdp + - const: vbif clocks: items: - - description: Display AHB clock from gcc - - description: Display AXI clock - - description: Display core clock + - description: Display AXI clock from gcc + - description: Display AHB clock from dispcc + - description: Display core clock from dispcc + - description: Display lut clock from dispcc + - description: Display vsync clock from dispcc clock-names: items: - - const: iface - const: bus + - const: iface - const: core + - const: lut + - const: vsync interrupts: maxItems: 1 - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": -const: 1 - - iommus: -items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port1 - - ranges: true - - interconnects: -items: - - description: Interconnect path specifying the port ids for data bus - - interconnect-names: -const: mdp0-mem + power-domains: +maxItems: 1 - resets: -items: - - description: MDSS_CORE reset + operating-points-v2: true -patternProperties: - "^display-controller@[0-9a-f]+$": -type: object -description: Node containing the properties of DPU. + ports: +$ref: /schemas/graph.yaml#/properties/ports +description: | + Contains the list of output ports from DPU device. These ports + connect to interfaces that are external to the DPU hardware, + such as DSI. Each output port contains an endpoint that + describes how it is connected to an external interface. properties: - compatible: -items: - - const: qcom,qcm2290-dpu - - reg: -items: - - description: Address offset and size for mdp register set - - description: Address offset and size for vbif register set - - reg-names: -items: - - const: mdp - - const: vbif - - clocks: -items: - - description: Display AXI clock from gcc - - description: Display AHB clock from dispcc - - description: Display core clock from dispcc - - description: Display lut clock from dispcc - - description: Display vsync clock from dispcc - - clock-names: -items: - - const: bus - - const: iface - - const: core - - const: lut - - const: vsync - - interrupts: -maxItems: 1 - - power-domains: -maxItems: 1 - - operating-points-v2: true - - ports: -$ref: /schemas/graph.yaml#/properties/ports -description: | - Contains the list of output ports from DPU device. These ports - connect to interfaces that are external to the DPU hardware, - such as DSI. Each output port contains an endpoint that - describes how it is connected to an external interface. - -properties: - port@0: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF1 (DSI1) - -required: - - port@0 + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF1 (DSI1) required: - - compatible - - reg - - reg-names - - clocks - - interrupts - - power-domains - - operating-points-v2 - - ports + - port@0 required: - compatible - reg - reg-names - - p
[PATCH v3 2/9] dt-bindings: display/msm: move qcom, sdm845-mdss schema to mdss.yaml
Move schema for qcom,sdm845-mdss from dpu-sdm845.yaml to mdss.yaml so that the dpu file describes only the DPU schema. Reviewed-by: Rob Herring Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dpu-sdm845.yaml | 135 --- .../devicetree/bindings/display/msm/mdss.yaml | 156 ++ 2 files changed, 160 insertions(+), 131 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml index 2bb8896beffc..2074e954372f 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml @@ -10,139 +10,74 @@ maintainers: - Krishna Manikandan description: | - Device tree bindings for MSM Mobile Display Subsystem(MDSS) that encapsulates - sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree - bindings of MDSS and DPU are mentioned for SDM845 target. + Device tree bindings for the DPU display controller for SDM845 target. properties: compatible: items: - - const: qcom,sdm845-mdss + - const: qcom,sdm845-dpu reg: -maxItems: 1 +items: + - description: Address offset and size for mdp register set + - description: Address offset and size for vbif register set reg-names: -const: mdss - - power-domains: -maxItems: 1 +items: + - const: mdp + - const: vbif clocks: items: - - description: Display AHB clock from gcc + - description: Display ahb clock + - description: Display axi clock - description: Display core clock + - description: Display vsync clock clock-names: items: - const: iface + - const: bus - const: core + - const: vsync interrupts: maxItems: 1 - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": -const: 1 - - iommus: -items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port1 - - ranges: true - - resets: -items: - - description: MDSS_CORE reset + power-domains: +maxItems: 1 -patternProperties: - "^display-controller@[0-9a-f]+$": -type: object -description: Node containing the properties of DPU. + operating-points-v2: true + ports: +$ref: /schemas/graph.yaml#/properties/ports +description: | + Contains the list of output ports from DPU device. These ports + connect to interfaces that are external to the DPU hardware, + such as DSI, DP etc. Each output port contains an endpoint that + describes how it is connected to an external interface. properties: - compatible: -items: - - const: qcom,sdm845-dpu - - reg: -items: - - description: Address offset and size for mdp register set - - description: Address offset and size for vbif register set - - reg-names: -items: - - const: mdp - - const: vbif - - clocks: -items: - - description: Display ahb clock - - description: Display axi clock - - description: Display core clock - - description: Display vsync clock - - clock-names: -items: - - const: iface - - const: bus - - const: core - - const: vsync - - interrupts: -maxItems: 1 - - power-domains: -maxItems: 1 - - operating-points-v2: true - ports: -$ref: /schemas/graph.yaml#/properties/ports -description: | - Contains the list of output ports from DPU device. These ports - connect to interfaces that are external to the DPU hardware, - such as DSI, DP etc. Each output port contains an endpoint that - describes how it is connected to an external interface. - -properties: - port@0: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF1 (DSI1) - - port@1: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF2 (DSI2) - -required: - - port@0 - - port@1 + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF1 (DSI1) + + port@1: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF2 (DSI2) required: - - compatible - - reg - - reg-names - - clocks - - interrupts - - power-domains - - operating-points-v2 - - ports + - port@0 + - port@1 required: - compatible - reg - reg-names - - power-domains - clocks - interrupts - - interrupt-controller - - iommus - - ranges + - power-domains + - operating-points-v2 + - ports additionalProperties: false diff --git a/Do
[PATCH v3 7/9] dt-bindings: display/mdm: add gcc-bus clock to dpu-smd845
Add gcc-bus clock required for the SDM845 DPU device tree node. This
change was made in the commit 111c52854102 ("arm64: dts: qcom: sdm845:
move bus clock to mdp node for sdm845 target"), but was not reflected in
the schema.
Reviewed-by: Rob Herring
Signed-off-by: Dmitry Baryshkov
---
.../devicetree/bindings/display/msm/dpu-sdm845.yaml| 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
index 2074e954372f..42ff85e80f45 100644
--- a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
@@ -29,6 +29,7 @@ properties:
clocks:
items:
+ - description: Display GCC bus clock
- description: Display ahb clock
- description: Display axi clock
- description: Display core clock
@@ -36,6 +37,7 @@ properties:
clock-names:
items:
+ - const: gcc-bus
- const: iface
- const: bus
- const: core
@@ -114,11 +116,12 @@ examples:
<0x0aeb 0x2008>;
reg-names = "mdp", "vbif";
-clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+clocks = <&gcc GCC_DISP_AXI_CLK>,
+ <&dispcc DISP_CC_MDSS_AHB_CLK>,
<&dispcc DISP_CC_MDSS_AXI_CLK>,
<&dispcc DISP_CC_MDSS_MDP_CLK>,
<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
-clock-names = "iface", "bus", "core", "vsync";
+clock-names = "gcc-bus", "iface", "bus", "core", "vsync";
interrupt-parent = <&mdss>;
interrupts = <0>;
--
2.35.1
[PATCH v3 8/9] dt-bindings: display/msm: move common DPU properties to dpu-common.yaml
Move properties common to all DPU DT nodes to the dpu-common.yaml. Note, this removes description of individual DPU port@ nodes. However such definitions add no additional value. The reg values do not correspond to hardware INTF indices. The driver discovers and binds these ports not paying any care for the order of these items. Thus just leave the reference to graph.yaml#/properties/ports and the description. Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dpu-common.yaml | 42 ++ .../bindings/display/msm/dpu-msm8998.yaml | 43 ++- .../bindings/display/msm/dpu-qcm2290.yaml | 39 ++--- .../bindings/display/msm/dpu-sc7180.yaml | 43 ++- .../bindings/display/msm/dpu-sc7280.yaml | 43 ++- .../bindings/display/msm/dpu-sdm845.yaml | 43 ++- 6 files changed, 62 insertions(+), 191 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/dpu-common.yaml diff --git a/Documentation/devicetree/bindings/display/msm/dpu-common.yaml b/Documentation/devicetree/bindings/display/msm/dpu-common.yaml new file mode 100644 index ..14eda883e149 --- /dev/null +++ b/Documentation/devicetree/bindings/display/msm/dpu-common.yaml @@ -0,0 +1,42 @@ +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/dpu-common.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Display DPU dt properties (common properties) + +maintainers: + - Dmitry Baryshkov + - Krishna Manikandan + - Rob Clark + +description: | + Common properties for QCom DPU display controller. + +properties: + interrupts: +maxItems: 1 + + power-domains: +maxItems: 1 + + operating-points-v2: true + + ports: +$ref: /schemas/graph.yaml#/properties/ports +description: | + Contains the list of output ports from DPU device. These ports + connect to interfaces that are external to the DPU hardware, + such as DSI, DP etc. + +required: + - compatible + - reg + - reg-names + - clocks + - interrupts + - power-domains + - operating-points-v2 + - ports + +additionalProperties: true diff --git a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml index 5caf46a1dd88..158bd93a157f 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml @@ -47,45 +47,10 @@ properties: - const: core - const: vsync - interrupts: -maxItems: 1 - - power-domains: -maxItems: 1 - - operating-points-v2: true - ports: -$ref: /schemas/graph.yaml#/properties/ports -description: | - Contains the list of output ports from DPU device. These ports - connect to interfaces that are external to the DPU hardware, - such as DSI, DP etc. Each output port contains an endpoint that - describes how it is connected to an external interface. - -properties: - port@0: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF1 (DSI1) - - port@1: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF2 (DSI2) - -required: - - port@0 - - port@1 - -required: - - compatible - - reg - - reg-names - - clocks - - interrupts - - power-domains - - operating-points-v2 - - ports - -additionalProperties: false +allOf: + - $ref: "/schemas/display/msm/dpu-common.yaml#" + +unevaluatedProperties: false examples: - | diff --git a/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml b/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml index 8027319b1aad..0364261bf3d2 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-qcm2290.yaml @@ -43,41 +43,10 @@ properties: - const: lut - const: vsync - interrupts: -maxItems: 1 - - power-domains: -maxItems: 1 - - operating-points-v2: true - - ports: -$ref: /schemas/graph.yaml#/properties/ports -description: | - Contains the list of output ports from DPU device. These ports - connect to interfaces that are external to the DPU hardware, - such as DSI. Each output port contains an endpoint that - describes how it is connected to an external interface. - -properties: - port@0: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF1 (DSI1) - -required: - - port@0 - -required: - - compatible - - reg - - reg-names - - clocks - - interrupts - - power-domains - - operating-points-v2 - - ports - -additionalProperties: false +allOf: + - $ref: "/schemas/display/msm/dpu-common.yaml#" + +unevaluatedProperties: false examples: - | diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml b/Documentation/devicetree/bindings/display/
[PATCH v3 0/9] dt-bindings: display/msm: rework MDSS and DPU bindings
Create separate YAML schema for MDSS devicesd$ (both for MDP5 and DPU devices). Cleanup DPU schema files, so that they do not contain schema for both MDSS and DPU nodes. Apply misc small fixes to the DPU schema afterwards. Changes since v2: - Added a patch to allow opp-table under the dpu* nodes. - Removed the c&p issue which allowed the @0 nodes under the MDSS device node. Changes since v1: - Renamed DPU device nodes from mdp@ to display-controller@ - Described removal of mistakenly mentioned "lut" clock - Switched mdss.yaml to use $ref instead of fixing compatible strings - Dropped mdp-opp-table description (renamed by Krzysztof in his patchset) - Reworked DPU's ports definitions. Dropped description of individual ports, left only /ports $ref and description in dpu-common.yaml. Dmitry Baryshkov (9): dt-bindings: display/msm: split qcom,mdss bindings dt-bindings: display/msm: move qcom,sdm845-mdss schema to mdss.yaml dt-bindings: display/msm: move qcom,sc7180-mdss schema to mdss.yaml dt-bindings: display/msm: move qcom,sc7280-mdss schema to mdss.yaml dt-bindings: display/msm: move qcom,qcm2290-mdss schema to mdss.yaml dt-bindings: display/msm: move qcom,msm8998-mdss schema to mdss.yaml dt-bindings: display/mdm: add gcc-bus clock to dpu-smd845 dt-bindings: display/msm: move common DPU properties to dpu-common.yaml dt-bindings: display/msm/dpu-common: add opp-table property .../bindings/display/msm/dpu-common.yaml | 45 +++ .../bindings/display/msm/dpu-msm8998.yaml | 139 +-- .../bindings/display/msm/dpu-qcm2290.yaml | 143 +-- .../bindings/display/msm/dpu-sc7180.yaml | 148 +-- .../bindings/display/msm/dpu-sc7280.yaml | 147 +-- .../bindings/display/msm/dpu-sdm845.yaml | 139 +-- .../devicetree/bindings/display/msm/mdp5.txt | 30 +- .../devicetree/bindings/display/msm/mdss.yaml | 361 ++ 8 files changed, 508 insertions(+), 644 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/dpu-common.yaml create mode 100644 Documentation/devicetree/bindings/display/msm/mdss.yaml -- 2.35.1
Re: [PATCH v2 11/11] dt-bindings: display/msm: move common DPU properties to dpu-common.yaml
On 18/07/2022 20:50, Rob Herring wrote: On Sun, Jul 10, 2022 at 12:00:40PM +0300, Dmitry Baryshkov wrote: Move properties common to all DPU DT nodes to the dpu-common.yaml. Note, this removes description of individual DPU port@ nodes. However such definitions add no additional value. The reg values do not correspond to hardware INTF indices. The driver discovers and binds these ports not paying any care for the order of these items. Thus just leave the reference to graph.yaml#/properties/ports and the description. Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dpu-common.yaml | 42 ++ .../bindings/display/msm/dpu-msm8998.yaml | 43 ++- .../bindings/display/msm/dpu-qcm2290.yaml | 39 ++--- .../bindings/display/msm/dpu-sc7180.yaml | 43 ++- .../bindings/display/msm/dpu-sc7280.yaml | 43 ++- .../bindings/display/msm/dpu-sdm845.yaml | 43 ++- 6 files changed, 62 insertions(+), 191 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/dpu-common.yaml diff --git a/Documentation/devicetree/bindings/display/msm/dpu-common.yaml b/Documentation/devicetree/bindings/display/msm/dpu-common.yaml new file mode 100644 index ..14eda883e149 --- /dev/null +++ b/Documentation/devicetree/bindings/display/msm/dpu-common.yaml @@ -0,0 +1,42 @@ +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/msm/dpu-common.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Display DPU dt properties (common properties) + +maintainers: + - Dmitry Baryshkov + - Krishna Manikandan + - Rob Clark + +description: | + Common properties for QCom DPU display controller. + +properties: + interrupts: +maxItems: 1 + + power-domains: +maxItems: 1 + + operating-points-v2: true + + ports: +$ref: /schemas/graph.yaml#/properties/ports +description: | + Contains the list of output ports from DPU device. These ports + connect to interfaces that are external to the DPU hardware, + such as DSI, DP etc. From the last version: In case of MDSS all ports are output, they are connected to the external interfaces (DSI, DP, HDMI, etc). The driver uses them to bind available interfaces (using components framework). The reg property of the port is completely ignored. It doesn't matter what the driver does or doesn't do. Without describing port nodes at all, you are not validating what port nodes can contain. Just try adding any property under a port node. You need at least: '^port@[0-N]$': $ref: graph.yaml#/properties/port Hmm, the graph.yaml already restricts the ports node to the ports@[0-9a-f]+$ + #address-cells/#size-cells. I don't think we have to add any additional restrictions/entries here. Do we? where N is the max number of ports. Rob -- With best wishes Dmitry
Re: [PATCH v7 1/8] overflow: Move and add few utility macros into overflow
On Tue, Aug 23, 2022 at 04:32:10AM +0900, Gwan-gyeong Mun wrote:
> On 8/22/22 11:05 PM, Andrzej Hajda wrote:
> > On 18.08.2022 02:12, Kees Cook wrote:
> > > On Thu, Aug 18, 2022 at 01:07:29AM +0200, Andi Shyti wrote:
> > > > [...]
> > > > > +#define safe_conversion(ptr, value) ({ \
> > > > > + typeof(value) __v = (value); \
> > > > > + typeof(ptr) __ptr = (ptr); \
> > > > > + overflows_type(__v, *__ptr) ? 0 : ((*__ptr =
> > > > > (typeof(*__ptr))__v), 1); \
> > > > > +})
> > >
> > > I try to avoid "safe" as an adjective for interface names, since it
> > > doesn't really answer "safe from what?" This looks more like "assign, but
> > > zero when out of bounds". And it can be built from existing macros here:
> > >
> > > if (check_add_overflow(0, value, ptr))
> > > *ptr = 0;
> > >
> > > I actually want to push back on this a bit, because there can still be
> > > logic bugs built around this kind of primitive. Shouldn't out-of-bounds
> > > assignments be seen as a direct failure? I would think this would be
> > > sufficient:
> > >
> > > #define check_assign(value, ptr) check_add_overflow(0, value, ptr)
> > >
> > > And callers would do:
> > >
> > > if (check_assign(value, &var))
> > > return -EINVAL;
> > >
> Yes, I also like check_assign() you suggested more than safe_conversion.
> As shown below, it would be more readable to return true when assign
> succeeds and false when it fails. What do you think?
No, this inverts the style of all the other check_*() functions, so it
should remain "non-zero is failure".
> /**
> * check_assign - perform a type conversion (cast) of an source value into
> * a new variable, checking that the destination is large enough to hold the
> * source value.
> *
> * @value: Source value
> * @ptr: Destination pointer address, If the pointer type is not used, a
> warning message is output during build.
> *
> * Returns:
> * If the value would overflow the destination, it returns false. If not
> return true.
> */
> #define check_assign(value, ptr) __must_check_overflow(({ \
> typecheck_pointer(ptr); \
> !__builtin_add_overflow(0, value, ptr); \
> }))
Please don't use the __builtin*s, instead stick to the check_* family,
as they correctly wrap the builtins and perform type checking, etc. As
mentioned, check_assign() should just be:
#define check_assign(value, ptr) check_add_overflow(0, value, ptr)
I don't think any of the other code is needed? What's the use-case for
the other stuff? i.e. Why does anything need overflows_type()?
-Kees
--
Kees Cook
Re: [PATCH -next] video: fbdev: chipsfb: add missing pci_disable_device() in chipsfb_pci_init()
On 8/19/22 10:57, Yang Yingliang wrote: > Add missing pci_disable_device() in error path in chipsfb_pci_init(). > > Signed-off-by: Yang Yingliang applied. Thanks! Helge > --- > drivers/video/fbdev/chipsfb.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c > index 393894af26f8..2b00a9d554fc 100644 > --- a/drivers/video/fbdev/chipsfb.c > +++ b/drivers/video/fbdev/chipsfb.c > @@ -430,6 +430,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const > struct pci_device_id *ent) > err_release_fb: > framebuffer_release(p); > err_disable: > + pci_disable_device(dp); > err_out: > return rc; > }
[PATCH] drm/sced: Add FIFO policy for scheduler rq
Poblem: Given many entities competing for same rq on
same scheduler an uncceptabliy long wait time for some
jobs waiting stuck in rq before being picked up are
observed (seen using GPUVis).
The issue is due to Round Robin policy used by scheduler
to pick up the next entity for execution. Under stress
of many entities and long job queus within entity some
jobs could be stack for very long time in it's entity's
queue before being popped from the queue and executed
while for other entites with samller job queues a job
might execute ealier even though that job arrived later
then the job in the long queue.
Fix:
Add FIFO selection policy to entites in RQ, chose next enitity
on rq in such order that if job on one entity arrived
ealrier then job on another entity the first job will start
executing ealier regardless of the length of the entity's job
queue.
Signed-off-by: Andrey Grodzovsky
Tested-by: Li Yunxiang (Teddy)
---
drivers/gpu/drm/scheduler/sched_entity.c | 2 +
drivers/gpu/drm/scheduler/sched_main.c | 65 ++--
include/drm/gpu_scheduler.h | 8 +++
3 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index 6b25b2f4f5a3..3bb7f69306ef 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -507,6 +507,8 @@ void drm_sched_entity_push_job(struct drm_sched_job
*sched_job)
atomic_inc(entity->rq->sched->score);
WRITE_ONCE(entity->last_user, current->group_leader);
first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
+ sched_job->submit_ts = ktime_get();
+
/* first job wakes up scheduler */
if (first) {
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 68317d3a7a27..c123aa120d06 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -59,6 +59,19 @@
#define CREATE_TRACE_POINTS
#include "gpu_scheduler_trace.h"
+
+
+int drm_sched_policy = -1;
+
+/**
+ * DOC: sched_policy (int)
+ * Used to override default entites scheduling policy in a run queue.
+ */
+MODULE_PARM_DESC(sched_policy,
+ "specify schedule policy for entites on a runqueue (-1 =
auto(default) value, 0 = Round Robin,1 = use FIFO");
+module_param_named(sched_policy, drm_sched_policy, int, 0444);
+
+
#define to_drm_sched_job(sched_job)\
container_of((sched_job), struct drm_sched_job, queue_node)
@@ -120,14 +133,16 @@ void drm_sched_rq_remove_entity(struct drm_sched_rq *rq,
}
/**
- * drm_sched_rq_select_entity - Select an entity which could provide a job to
run
+ * drm_sched_rq_select_entity_rr - Select an entity which could provide a job
to run
*
* @rq: scheduler run queue to check.
*
- * Try to find a ready entity, returns NULL if none found.
+ * Try to find a ready entity, in round robin manner.
+ *
+ * Returns NULL if none found.
*/
static struct drm_sched_entity *
-drm_sched_rq_select_entity(struct drm_sched_rq *rq)
+drm_sched_rq_select_entity_rr(struct drm_sched_rq *rq)
{
struct drm_sched_entity *entity;
@@ -163,6 +178,45 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
return NULL;
}
+/**
+ * drm_sched_rq_select_entity_fifo - Select an entity which could provide a
job to run
+ *
+ * @rq: scheduler run queue to check.
+ *
+ * Try to find a ready entity, based on FIFO order of jobs arrivals.
+ *
+ * Returns NULL if none found.
+ */
+static struct drm_sched_entity *
+drm_sched_rq_select_entity_fifo(struct drm_sched_rq *rq)
+{
+ struct drm_sched_entity *tmp, *entity = NULL;
+ ktime_t oldest_ts = KTIME_MAX;
+ struct drm_sched_job *sched_job;
+
+ spin_lock(&rq->lock);
+
+ list_for_each_entry(tmp, &rq->entities, list) {
+
+ if (drm_sched_entity_is_ready(tmp)) {
+ sched_job =
to_drm_sched_job(spsc_queue_peek(&tmp->job_queue));
+
+ if (ktime_before(sched_job->submit_ts, oldest_ts)) {
+ oldest_ts = sched_job->submit_ts;
+ entity = tmp;
+ }
+ }
+ }
+
+ if (entity) {
+ rq->current_entity = entity;
+ reinit_completion(&entity->entity_idle);
+ }
+
+ spin_unlock(&rq->lock);
+ return entity;
+}
+
/**
* drm_sched_job_done - complete a job
* @s_job: pointer to the job which is done
@@ -804,7 +858,10 @@ drm_sched_select_entity(struct drm_gpu_scheduler *sched)
/* Kernel run queue has higher priority than normal run queue*/
for (i = DRM_SCHED_PRIORITY_COUNT - 1; i >= DRM_SCHED_PRIORITY_MIN;
i--) {
- entity = drm_sched_rq_select_entity(&sched->sched_rq[i]);
+ entity = drm_sched_policy != 1 ?
+
drm_sched_rq_select_entity_rr
Re: [PATCH] fbcon: Destroy mutex on freeing struct fb_info
On 8/21/22 13:17, Shigeru Yoshida wrote: > It's needed to destroy bl_curve_mutex on freeing struct fb_info since > the mutex is embedded in the structure and initialized when it's > allocated. > > Signed-off-by: Shigeru Yoshida applied. Thanks, Helge > --- > drivers/video/fbdev/core/fbsysfs.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/video/fbdev/core/fbsysfs.c > b/drivers/video/fbdev/core/fbsysfs.c > index c2a60b187467..4d7f63892dcc 100644 > --- a/drivers/video/fbdev/core/fbsysfs.c > +++ b/drivers/video/fbdev/core/fbsysfs.c > @@ -84,6 +84,10 @@ void framebuffer_release(struct fb_info *info) > if (WARN_ON(refcount_read(&info->count))) > return; > > +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) > + mutex_destroy(&info->bl_curve_mutex); > +#endif > + > kfree(info->apertures); > kfree(info); > }
Re: [PATCH] video: fbdev: radeon: Clean up some inconsistent indenting
On 8/19/22 13:06, Jiapeng Chong wrote:
> No functional modification involved.
>
> drivers/video/fbdev/aty/radeon_base.c:2107 radeon_identify_vram() warn:
> inconsistent indenting.
>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=1932
> Reported-by: Abaci Robot
> Signed-off-by: Jiapeng Chong
applied.
Thanks!
Helge
> ---
> drivers/video/fbdev/aty/radeon_base.c | 46 +--
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/video/fbdev/aty/radeon_base.c
> b/drivers/video/fbdev/aty/radeon_base.c
> index 0a8199985d52..1e6ac7ef3e73 100644
> --- a/drivers/video/fbdev/aty/radeon_base.c
> +++ b/drivers/video/fbdev/aty/radeon_base.c
> @@ -2095,34 +2095,34 @@ static void radeon_identify_vram(struct radeonfb_info
> *rinfo)
> u32 tmp;
>
> /* framebuffer size */
> -if ((rinfo->family == CHIP_FAMILY_RS100) ||
> + if ((rinfo->family == CHIP_FAMILY_RS100) ||
> (rinfo->family == CHIP_FAMILY_RS200) ||
> (rinfo->family == CHIP_FAMILY_RS300) ||
> (rinfo->family == CHIP_FAMILY_RC410) ||
> (rinfo->family == CHIP_FAMILY_RS400) ||
> (rinfo->family == CHIP_FAMILY_RS480) ) {
> - u32 tom = INREG(NB_TOM);
> - tmp = tom >> 16) - (tom & 0x) + 1) << 6) * 1024);
> -
> - radeon_fifo_wait(6);
> - OUTREG(MC_FB_LOCATION, tom);
> - OUTREG(DISPLAY_BASE_ADDR, (tom & 0x) << 16);
> - OUTREG(CRTC2_DISPLAY_BASE_ADDR, (tom & 0x) << 16);
> - OUTREG(OV0_BASE_ADDR, (tom & 0x) << 16);
> -
> - /* This is supposed to fix the crtc2 noise problem. */
> - OUTREG(GRPH2_BUFFER_CNTL, INREG(GRPH2_BUFFER_CNTL) & ~0x7f);
> -
> - if ((rinfo->family == CHIP_FAMILY_RS100) ||
> - (rinfo->family == CHIP_FAMILY_RS200)) {
> - /* This is to workaround the asic bug for RMX, some versions
> -of BIOS doesn't have this register initialized correctly.
> - */
> - OUTREGP(CRTC_MORE_CNTL, CRTC_H_CUTOFF_ACTIVE_EN,
> - ~CRTC_H_CUTOFF_ACTIVE_EN);
> - }
> -} else {
> - tmp = INREG(CNFG_MEMSIZE);
> + u32 tom = INREG(NB_TOM);
> +
> + tmp = tom >> 16) - (tom & 0x) + 1) << 6) * 1024);
> + radeon_fifo_wait(6);
> + OUTREG(MC_FB_LOCATION, tom);
> + OUTREG(DISPLAY_BASE_ADDR, (tom & 0x) << 16);
> + OUTREG(CRTC2_DISPLAY_BASE_ADDR, (tom & 0x) << 16);
> + OUTREG(OV0_BASE_ADDR, (tom & 0x) << 16);
> +
> + /* This is supposed to fix the crtc2 noise problem. */
> + OUTREG(GRPH2_BUFFER_CNTL, INREG(GRPH2_BUFFER_CNTL) & ~0x7f);
> +
> + if ((rinfo->family == CHIP_FAMILY_RS100) ||
> + (rinfo->family == CHIP_FAMILY_RS200)) {
> + /* This is to workaround the asic bug for RMX, some
> versions
> + * of BIOS doesn't have this register initialized
> correctly.
> + */
> + OUTREGP(CRTC_MORE_CNTL, CRTC_H_CUTOFF_ACTIVE_EN,
> + ~CRTC_H_CUTOFF_ACTIVE_EN);
> + }
> + } else {
> + tmp = INREG(CNFG_MEMSIZE);
> }
>
> /* mem size is bits [28:0], mask off the rest */
Re: [PATCH] drm/amd/display: remove unneeded defines from bios parser
Applied. Thanks!
Alex
On Sun, Aug 21, 2022 at 2:41 AM Tales Aparecida
wrote:
>
> Removes DEFINEs that should have been removed after they were
> introduced to ObjectID.h by the commit abea57d70e90
> ("drm/amdgpu: Add BRACKET_LAYOUT_ENUMs to ObjectID.h")
>
> Signed-off-by: Tales Aparecida
> ---
> .../drm/amd/display/dc/bios/bios_parser2.c| 19 ---
> 1 file changed, 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
> b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
> index 09fbb7ad5362..ead4da11a992 100644
> --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
> +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
> @@ -44,25 +44,6 @@
>
> #include "bios_parser_common.h"
>
> -/* Temporarily add in defines until ObjectID.h patch is updated in a few
> days */
> -#ifndef GENERIC_OBJECT_ID_BRACKET_LAYOUT
> -#define GENERIC_OBJECT_ID_BRACKET_LAYOUT 0x05
> -#endif /* GENERIC_OBJECT_ID_BRACKET_LAYOUT */
> -
> -#ifndef GENERICOBJECT_BRACKET_LAYOUT_ENUM_ID1
> -#define GENERICOBJECT_BRACKET_LAYOUT_ENUM_ID1 \
> - (GRAPH_OBJECT_TYPE_GENERIC << OBJECT_TYPE_SHIFT |\
> - GRAPH_OBJECT_ENUM_ID1 << ENUM_ID_SHIFT |\
> - GENERIC_OBJECT_ID_BRACKET_LAYOUT << OBJECT_ID_SHIFT)
> -#endif /* GENERICOBJECT_BRACKET_LAYOUT_ENUM_ID1 */
> -
> -#ifndef GENERICOBJECT_BRACKET_LAYOUT_ENUM_ID2
> -#define GENERICOBJECT_BRACKET_LAYOUT_ENUM_ID2 \
> - (GRAPH_OBJECT_TYPE_GENERIC << OBJECT_TYPE_SHIFT |\
> - GRAPH_OBJECT_ENUM_ID2 << ENUM_ID_SHIFT |\
> - GENERIC_OBJECT_ID_BRACKET_LAYOUT << OBJECT_ID_SHIFT)
> -#endif /* GENERICOBJECT_BRACKET_LAYOUT_ENUM_ID2 */
> -
> #define DC_LOGGER \
> bp->base.ctx->logger
>
> --
> 2.37.2
>
Re: [PATCH] fb_pm2: Add a check to avoid potential divide by zero error
On 8/18/22 12:44, Letu Ren wrote:
> In `do_fb_ioctl()` of fbmem.c, if cmd is FBIOPUT_VSCREENINFO, var will be
> copied from user, then go through `fb_set_var()` and
> `info->fbops->fb_check_var()` which could may be `pm2fb_check_var()`.
> Along the path, `var->pixclock` won't be modified. This function checks
> whether reciprocal of `var->pixclock` is too high. If `var->pixclock` is
> zero, there will be a divide by zero error. So, it is necessary to check
> whether denominator is zero to avoid crash. As this bug is found by
> Syzkaller, logs are listed below.
>
> divide error in pm2fb_check_var
> Call Trace:
>
> fb_set_var+0x367/0xeb0 drivers/video/fbdev/core/fbmem.c:1015
> do_fb_ioctl+0x234/0x670 drivers/video/fbdev/core/fbmem.c:1110
> fb_ioctl+0xdd/0x130 drivers/video/fbdev/core/fbmem.c:1189
>
> Reported-by: Zheyu Ma
> Signed-off-by: Letu Ren
applied.
Thanks!
Helge
> ---
> drivers/video/fbdev/pm2fb.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c
> index d3be2c64f1c0..8fd79deb1e2a 100644
> --- a/drivers/video/fbdev/pm2fb.c
> +++ b/drivers/video/fbdev/pm2fb.c
> @@ -617,6 +617,11 @@ static int pm2fb_check_var(struct fb_var_screeninfo
> *var, struct fb_info *info)
> return -EINVAL;
> }
>
> + if (!var->pixclock) {
> + DPRINTK("pixclock is zero\n");
> + return -EINVAL;
> + }
> +
> if (PICOS2KHZ(var->pixclock) > PM2_MAX_PIXCLOCK) {
> DPRINTK("pixclock too high (%ldKHz)\n",
> PICOS2KHZ(var->pixclock));
Re: [PATCH] video: fbdev: sis_main: Clean up some inconsistent indenting
On 8/19/22 13:04, Jiapeng Chong wrote:
> No functional modification involved.
>
> drivers/video/fbdev/sis/sis_main.c:6165 sisfb_probe() warn: inconsistent
> indenting.
> drivers/video/fbdev/sis/sis_main.c:4266 sisfb_post_300_rwtest() warn:
> inconsistent indenting.
> drivers/video/fbdev/sis/sis_main.c:2388 SISDoSense() warn: inconsistent
> indenting.
> drivers/video/fbdev/sis/sis_main.c:2531 SiS_Sense30x() warn: inconsistent
> indenting.
> drivers/video/fbdev/sis/sis_main.c:2382 SISDoSense() warn: inconsistent
> indenting.
> drivers/video/fbdev/sis/sis_main.c:2250 sisfb_sense_crt1() warn: inconsistent
> indenting.
> drivers/video/fbdev/sis/sis_main.c:672 sisfb_validate_mode() warn:
> inconsistent indenting.
>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=1934
> Reported-by: Abaci Robot
> Signed-off-by: Jiapeng Chong
applied.
Thanks!
Helge
> ---
> drivers/video/fbdev/sis/sis_main.c | 274 +++--
> 1 file changed, 141 insertions(+), 133 deletions(-)
>
> diff --git a/drivers/video/fbdev/sis/sis_main.c
> b/drivers/video/fbdev/sis/sis_main.c
> index 7114c5c17c91..ac4680a74d78 100644
> --- a/drivers/video/fbdev/sis/sis_main.c
> +++ b/drivers/video/fbdev/sis/sis_main.c
> @@ -650,37 +650,37 @@ sisfb_validate_mode(struct sis_video_info *ivideo, int
> myindex, u32 vbflags)
> u16 xres=0, yres, myres;
>
> #ifdef CONFIG_FB_SIS_300
> - if(ivideo->sisvga_engine == SIS_300_VGA) {
> - if(!(sisbios_mode[myindex].chipset & MD_SIS300))
> + if (ivideo->sisvga_engine == SIS_300_VGA) {
> + if (!(sisbios_mode[myindex].chipset & MD_SIS300))
> return -1 ;
> }
> #endif
> #ifdef CONFIG_FB_SIS_315
> - if(ivideo->sisvga_engine == SIS_315_VGA) {
> - if(!(sisbios_mode[myindex].chipset & MD_SIS315))
> + if (ivideo->sisvga_engine == SIS_315_VGA) {
> + if (!(sisbios_mode[myindex].chipset & MD_SIS315))
> return -1;
> }
> #endif
>
> myres = sisbios_mode[myindex].yres;
>
> - switch(vbflags & VB_DISPTYPE_DISP2) {
> + switch (vbflags & VB_DISPTYPE_DISP2) {
>
> case CRT2_LCD:
> xres = ivideo->lcdxres; yres = ivideo->lcdyres;
>
> - if((ivideo->SiS_Pr.SiS_CustomT != CUT_PANEL848) &&
> -(ivideo->SiS_Pr.SiS_CustomT != CUT_PANEL856)) {
> - if(sisbios_mode[myindex].xres > xres)
> + if ((ivideo->SiS_Pr.SiS_CustomT != CUT_PANEL848) &&
> + (ivideo->SiS_Pr.SiS_CustomT != CUT_PANEL856)) {
> + if (sisbios_mode[myindex].xres > xres)
> return -1;
> - if(myres > yres)
> + if (myres > yres)
> return -1;
> }
>
> - if(ivideo->sisfb_fstn) {
> - if(sisbios_mode[myindex].xres == 320) {
> - if(myres == 240) {
> -
> switch(sisbios_mode[myindex].mode_no[1]) {
> + if (ivideo->sisfb_fstn) {
> + if (sisbios_mode[myindex].xres == 320) {
> + if (myres == 240) {
> + switch
> (sisbios_mode[myindex].mode_no[1]) {
> case 0x50: myindex =
> MODE_FSTN_8; break;
> case 0x56: myindex =
> MODE_FSTN_16; break;
> case 0x53: return -1;
> @@ -689,7 +689,7 @@ sisfb_validate_mode(struct sis_video_info *ivideo, int
> myindex, u32 vbflags)
> }
> }
>
> - if(SiS_GetModeID_LCD(ivideo->sisvga_engine, vbflags,
> sisbios_mode[myindex].xres,
> + if (SiS_GetModeID_LCD(ivideo->sisvga_engine, vbflags,
> sisbios_mode[myindex].xres,
> sisbios_mode[myindex].yres, 0,
> ivideo->sisfb_fstn,
> ivideo->SiS_Pr.SiS_CustomT, xres, yres,
> ivideo->vbflags2) < 0x14) {
> return -1;
> @@ -697,14 +697,14 @@ sisfb_validate_mode(struct sis_video_info *ivideo, int
> myindex, u32 vbflags)
> break;
>
> case CRT2_TV:
> - if(SiS_GetModeID_TV(ivideo->sisvga_engine, vbflags,
> sisbios_mode[myindex].xres,
> + if (SiS_GetModeID_TV(ivideo->sisvga_engine, vbflags,
> sisbios_mode[myindex].xres,
> sisbios_mode[myindex].yres, 0,
> ivideo->vbflags2) < 0x14) {
> return -1;
> }
> break;
>
> case CRT2_VGA:
> - if(SiS_GetModeID_VGA2(ivideo->sisvga_engine, vbflags,
> sisbios_mode[myindex].xres,
> + if (SiS_GetModeID_VGA2(ivideo->sisvga_engine, vbflags,
> sisbios_mode[myindex].xres,
> sisbios_mode[myindex].yres, 0,
> ivideo->vbflags2) < 0x14) {
>
Re: [RESEND v6 6/9] drm: vkms: Refactor the plane composer to accept new formats
On 8/22/22 16:01, Melissa Wen wrote:
On 08/22, Igor Matheus Andrade Torrente wrote:
Hi Melissa,
On 8/20/22 07:51, Melissa Wen wrote:
On 08/19, Igor Torrente wrote:
Currently the blend function only accepts XRGB_ and ARGB_
as a color input.
This patch refactors all the functions related to the plane composition
to overcome this limitation.
The pixels blend is done using the new internal format. And new handlers
are being added to convert a specific format to/from this internal format.
So the blend operation depends on these handlers to convert to this common
format. The blended result, if necessary, is converted to the writeback
buffer format.
This patch introduces three major differences to the blend function.
1 - All the planes are blended at once.
2 - The blend calculus is done as per line instead of per pixel.
3 - It is responsible to calculates the CRC and writing the writeback
buffer(if necessary).
These changes allow us to allocate way less memory in the intermediate
buffer to compute these operations. Because now we don't need to
have the entire intermediate image lines at once, just one line is
enough.
| Memory consumption (output dimensions) |
|:--:|
| Current | This patch|
|:--:|:-:|
| Width * Heigth | 2 * Width |
Beyond memory, we also have a minor performance benefit from all
these changes. Results running the IGT[1] test
`igt@kms_cursor_crc@pipe-a-cursor-512x512-onscreen` ten times:
| Frametime |
|:--:|
| Implementation | Current | This commit |
|:---:|:-:|::|
| frametime range | 9~22 ms |5~17 ms |
| Average | 11.4 ms |7.8 ms|
[1] IGT commit id: bc3f6833a12221a46659535dac06ebb312490eb4
V2: Improves the performance drastically, by performing the operations
per-line and not per-pixel(Pekka Paalanen).
Minor improvements(Pekka Paalanen).
V3: Changes the code to blend the planes all at once. This improves
performance, memory consumption, and removes much of the weirdness
of the V2(Pekka Paalanen and me).
Minor improvements(Pekka Paalanen and me).
V4: Rebase the code and adapt it to the new NUM_OVERLAY_PLANES constant.
V5: Minor checkpatch fixes and the removal of TO-DO item(Melissa Wen).
Several security/robustness improvents(Pekka Paalanen).
Removes check_planes_x_bounds function and allows partial
partly off-screen(Pekka Paalanen).
V6: Fix a mismatch of some variable sizes (Pekka Paalanen).
Several minor improvements (Pekka Paalanen).
Reported-by: kernel test robot
Signed-off-by: Igor Torrente
---
Documentation/gpu/vkms.rst| 4 -
drivers/gpu/drm/vkms/Makefile | 1 +
drivers/gpu/drm/vkms/vkms_composer.c | 320 --
drivers/gpu/drm/vkms/vkms_formats.c | 155 +
drivers/gpu/drm/vkms/vkms_formats.h | 12 +
drivers/gpu/drm/vkms/vkms_plane.c | 3 +
drivers/gpu/drm/vkms/vkms_writeback.c | 3 +
7 files changed, 317 insertions(+), 181 deletions(-)
create mode 100644 drivers/gpu/drm/vkms/vkms_formats.c
create mode 100644 drivers/gpu/drm/vkms/vkms_formats.h
diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 973e2d43108b..a49e4ae92653 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -118,10 +118,6 @@ Add Plane Features
There's lots of plane features we could add support for:
-- Clearing primary plane: clear primary plane before plane composition (at the
- start) for correctness of pixel blend ops. It also guarantees alpha channel
- is cleared in the target buffer for stable crc. [Good to get started]
-
- ARGB format on primary plane: blend the primary plane into background with
translucent alpha.
diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
index 72f779cbfedd..1b28a6a32948 100644
--- a/drivers/gpu/drm/vkms/Makefile
+++ b/drivers/gpu/drm/vkms/Makefile
@@ -3,6 +3,7 @@ vkms-y := \
vkms_drv.o \
vkms_plane.o \
vkms_output.o \
+ vkms_formats.o \
vkms_crtc.o \
vkms_composer.o \
vkms_writeback.o
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index b9fb408e8973..5b1a8bdd8268 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -7,204 +7,188 @@
#include
#include
#include
+#include
#include "vkms_drv.h"
-static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer,
-const struct vkms_frame_info *frame_info)
+static u16 pre_mul_blend_channel(u16 src, u16 dst, u16 alpha)
{
- u32 pixel;
- int src_offset = frame_info->offset + (y * frame_info->pitch)
- + (x * frame_info->cpp);
+
Re: [RESEND v6 2/9] drm: vkms: Rename `vkms_composer` to `vkms_frame_info`
On 8/22/22 15:37, Melissa Wen wrote:
On 08/22, Igor Matheus Andrade Torrente wrote:
Hi Mellisa,
On 8/20/22 08:00, Melissa Wen wrote:
On 08/19, Igor Torrente wrote:
Changes the name of this struct to a more meaningful name.
A name that represents better what this struct is about.
Composer is the code that do the compositing of the planes.
This struct contains information on the frame used in the output
composition. Thus, vkms_frame_info is a better name to represent
this.
V5: Fix a commit message typo(Melissa Wen).
Reviewed-by: Melissa Wen
Signed-off-by: Igor Torrente
---
drivers/gpu/drm/vkms/vkms_composer.c | 87 ++--
drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
drivers/gpu/drm/vkms/vkms_plane.c| 38 ++--
3 files changed, 66 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 775b97766e08..0aded4e87e60 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -11,11 +11,11 @@
#include "vkms_drv.h"
static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer,
-const struct vkms_composer *composer)
+const struct vkms_frame_info *frame_info)
{
u32 pixel;
- int src_offset = composer->offset + (y * composer->pitch)
- + (x * composer->cpp);
+ int src_offset = frame_info->offset + (y * frame_info->pitch)
+ + (x * frame_info->cpp);
pixel = *(u32 *)&buffer[src_offset];
@@ -26,24 +26,24 @@ static u32 get_pixel_from_buffer(int x, int y, const u8
*buffer,
* compute_crc - Compute CRC value on output frame
*
* @vaddr: address to final framebuffer
- * @composer: framebuffer's metadata
+ * @frame_info: framebuffer's metadata
*
* returns CRC value computed using crc32 on the visible portion of
* the final framebuffer at vaddr_out
*/
static uint32_t compute_crc(const u8 *vaddr,
- const struct vkms_composer *composer)
+ const struct vkms_frame_info *frame_info)
{
int x, y;
u32 crc = 0, pixel = 0;
- int x_src = composer->src.x1 >> 16;
- int y_src = composer->src.y1 >> 16;
- int h_src = drm_rect_height(&composer->src) >> 16;
- int w_src = drm_rect_width(&composer->src) >> 16;
+ int x_src = frame_info->src.x1 >> 16;
+ int y_src = frame_info->src.y1 >> 16;
+ int h_src = drm_rect_height(&frame_info->src) >> 16;
+ int w_src = drm_rect_width(&frame_info->src) >> 16;
for (y = y_src; y < y_src + h_src; ++y) {
for (x = x_src; x < x_src + w_src; ++x) {
- pixel = get_pixel_from_buffer(x, y, vaddr, composer);
+ pixel = get_pixel_from_buffer(x, y, vaddr, frame_info);
crc = crc32_le(crc, (void *)&pixel, sizeof(u32));
}
}
@@ -98,8 +98,8 @@ static void x_blend(const u8 *xrgb_src, u8 *xrgb_dst)
* blend - blend value at vaddr_src with value at vaddr_dst
* @vaddr_dst: destination address
* @vaddr_src: source address
- * @dst_composer: destination framebuffer's metadata
- * @src_composer: source framebuffer's metadata
+ * @dst_frame_info: destination framebuffer's metadata
+ * @src_frame_info: source framebuffer's metadata
* @pixel_blend: blending equation based on plane format
*
* Blend the vaddr_src value with the vaddr_dst value using a pixel blend
@@ -111,33 +111,33 @@ static void x_blend(const u8 *xrgb_src, u8 *xrgb_dst)
* pixel color values
*/
static void blend(void *vaddr_dst, void *vaddr_src,
- struct vkms_composer *dst_composer,
- struct vkms_composer *src_composer,
+ struct vkms_frame_info *dst_frame_info,
+ struct vkms_frame_info *src_frame_info,
void (*pixel_blend)(const u8 *, u8 *))
{
int i, j, j_dst, i_dst;
int offset_src, offset_dst;
u8 *pixel_dst, *pixel_src;
- int x_src = src_composer->src.x1 >> 16;
- int y_src = src_composer->src.y1 >> 16;
+ int x_src = src_frame_info->src.x1 >> 16;
+ int y_src = src_frame_info->src.y1 >> 16;
- int x_dst = src_composer->dst.x1;
- int y_dst = src_composer->dst.y1;
- int h_dst = drm_rect_height(&src_composer->dst);
- int w_dst = drm_rect_width(&src_composer->dst);
+ int x_dst = src_frame_info->dst.x1;
+ int y_dst = src_frame_info->dst.y1;
+ int h_dst = drm_rect_height(&src_frame_info->dst);
+ int w_dst = drm_rect_width(&src_frame_info->dst);
int y_limit = y_src + h_dst;
int x_limit = x_src + w_dst;
for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
- offset_dst = ds
Re: [PATCH v7 1/8] overflow: Move and add few utility macros into overflow
On 8/22/22 11:05 PM, Andrzej Hajda wrote:
On 18.08.2022 02:12, Kees Cook wrote:
On Thu, Aug 18, 2022 at 01:07:29AM +0200, Andi Shyti wrote:
Hi Kees,
would you mind taking a look at this patch?
Hi! Thanks for the heads-up!
Thanks,
Andi
On Tue, Aug 16, 2022 at 06:35:18PM +0900, Gwan-gyeong Mun wrote:
It moves overflows_type utility macro into overflow header from
i915_utils
header. The overflows_type can be used to catch the truncation
between data
types. And it adds safe_conversion() macro which performs a type
conversion
(cast) of an source value into a new variable, checking that the
destination is large enough to hold the source value. And the
functionality
of overflows_type has been improved to handle the signbit.
The is_unsigned_type macro has been added to check the sign bit of the
built-in type.
v3: Add is_type_unsigned() macro (Mauro)
Modify overflows_type() macro to consider signed data types
(Mauro)
Fix the problem that safe_conversion() macro always returns true
v4: Fix kernel-doc markups
v6: Move macro addition location so that it can be used by other
than drm
subsystem (Jani, Mauro, Andi)
Change is_type_unsigned to is_unsigned_type to have the same
name form
as is_signed_type macro
Signed-off-by: Gwan-gyeong Mun
Cc: Thomas Hellström
Cc: Matthew Auld
Cc: Nirmoy Das
Cc: Jani Nikula
Cc: Andi Shyti
Reviewed-by: Mauro Carvalho Chehab (v5)
---
drivers/gpu/drm/i915/i915_utils.h | 5 +--
include/linux/overflow.h | 54
+++
2 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_utils.h
b/drivers/gpu/drm/i915/i915_utils.h
index c10d68cdc3ca..eb0ded23fa9c 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_X86
#include
@@ -111,10 +112,6 @@ bool i915_error_injected(void);
#define range_overflows_end_t(type, start, size, max) \
range_overflows_end((type)(start), (type)(size), (type)(max))
-/* Note we don't consider signbits :| */
-#define overflows_type(x, T) \
- (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T))
-
#define ptr_mask_bits(ptr, n) ({ \
unsigned long __v = (unsigned long)(ptr); \
(typeof(ptr))(__v & -BIT(n)); \
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index f1221d11f8e5..462a03454377 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -35,6 +35,60 @@
#define type_max(T) ((T)((__type_half_max(T) - 1) +
__type_half_max(T)))
#define type_min(T) ((T)((T)-type_max(T)-(T)1))
+/**
+ * is_unsigned_type - helper for checking data type which is an
unsigned data
+ * type or not
+ * @x: The data type to check
+ *
+ * Returns:
+ * True if the data type is an unsigned data type, false otherwise.
+ */
+#define is_unsigned_type(x) ((typeof(x))-1 >= (typeof(x))0)
I'd rather not have separate logic for this. Instead, I'd like it to be:
#define is_unsigned_type(x) (!is_signed_type(x))
+
+/**
+ * overflows_type - helper for checking the truncation between data
types
+ * @x: Source for overflow type comparison
+ * @T: Destination for overflow type comparison
+ *
+ * It compares the values and size of each data type between the
first and
+ * second argument to check whether truncation can occur when
assigning the
+ * first argument to the variable of the second argument.
+ * Source and Destination can be used with or without sign bit.
+ * Composite data structures such as union and structure are not
considered.
+ * Enum data types are not considered.
+ * Floating point data types are not considered.
+ *
+ * Returns:
+ * True if truncation can occur, false otherwise.
+ */
+#define overflows_type(x, T) \
+ (is_unsigned_type(x) ? \
+ is_unsigned_type(T) ? \
+ (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T)) ? 1
: 0 \
+ : (sizeof(x) >= sizeof(T) && (x) >> (BITS_PER_TYPE(T) -
1)) ? 1 : 0 \
+ : is_unsigned_type(T) ? \
+ ((x) < 0) ? 1 : (sizeof(x) > sizeof(T) && (x) >>
BITS_PER_TYPE(T)) ? 1 : 0 \
+ : (sizeof(x) > sizeof(T)) ? \
+ ((x) < 0) ? (((x) * -1) >> BITS_PER_TYPE(T)) ? 1 : 0 \
+ : ((x) >> BITS_PER_TYPE(T)) ? 1 : 0 \
+ : 0)
Like the other, I'd much rather this was rephrased in terms of the
existing macros (e.g. type_min()/type_max().)
Thanks for all of your comments.
The version that implements overflows_type() using type_min() and
type_max() includes modifications to the following macros.
In implementations of is_signed_type(), __type_half_max(), type_max(),
type_min(), where types are used as variables, the addition of typeof()
is necessary.
And the operation was confirmed through previously shared test cases.
https://patchwork.freedesktop.org/patch/492374/?series=104704&rev=3
#define is_signed_type(x) (((typeof(x))(
RE: [PATCH v1 2/4] drm/hyperv: Don't forget to put PCI device when removing conflicting FB fails
From: Vitaly Kuznetsov Sent: Thursday, August 18, 2022
7:25 AM
>
> When drm_aperture_remove_conflicting_pci_framebuffers() fails, 'pdev'
> needs to be released with pci_dev_put().
>
> Fixes: 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video
> device")
> Signed-off-by: Vitaly Kuznetsov
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 46f6c454b820..ca4e517b95ca 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -82,7 +82,7 @@ static int hyperv_setup_gen1(struct hyperv_drm_device *hv)
> ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev,
> &hyperv_driver);
> if (ret) {
> drm_err(dev, "Not able to remove boot fb\n");
> - return ret;
> + goto error;
> }
>
> if (pci_request_region(pdev, 0, DRIVER_NAME) != 0)
> --
> 2.37.1
This patch appears to be obsoleted by commit a0ab5abced55
that was merged into 6.0-rc1. Of course, it does beg the question of
why the original function hyperv_setup_gen2(), which is now renamed
to hyperv_setup_vram(), doesn't check the return value from
drm_aperture_remove_conflicting_framebuffers().
Michael
RE: [PATCH v1 1/4] Drivers: hv: Move legacy Hyper-V PCI video device's ids to linux/hyperv.h
From: Vitaly Kuznetsov Sent: Thursday, August 18, 2022
7:25 AM
>
> There are already two places in kernel with PCI_VENDOR_ID_MICROSOFT/
> PCI_DEVICE_ID_HYPERV_VIDEO and there's a need to use these from core
> Vmbus code. Move the defines to a common header.
>
> No functional change.
>
> Signed-off-by: Vitaly Kuznetsov
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 3 ---
> drivers/video/fbdev/hyperv_fb.c | 4
> include/linux/hyperv.h | 4
> 3 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 4a8941fa0815..46f6c454b820 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -23,9 +23,6 @@
> #define DRIVER_MAJOR 1
> #define DRIVER_MINOR 0
>
> -#define PCI_VENDOR_ID_MICROSOFT 0x1414
> -#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
> -
> DEFINE_DRM_GEM_FOPS(hv_fops);
>
> static struct drm_driver hyperv_driver = {
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index 886c564787f1..b58b445bb529 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -74,10 +74,6 @@
> #define SYNTHVID_DEPTH_WIN8 32
> #define SYNTHVID_FB_SIZE_WIN8 (8 * 1024 * 1024)
>
> -#define PCI_VENDOR_ID_MICROSOFT 0x1414
> -#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
> -
> -
> enum pipe_msg_type {
> PIPE_MSG_INVALID,
> PIPE_MSG_DATA,
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 3b42264333ef..4bb39a8f1af7 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1516,6 +1516,10 @@ void vmbus_free_mmio(resource_size_t start,
> resource_size_t size);
> .guid = GUID_INIT(0xc376c1c3, 0xd276, 0x48d2, 0x90, 0xa9, \
> 0xc0, 0x47, 0x48, 0x07, 0x2c, 0x60)
>
> +/* Legacy Hyper-V PCI video device */
> +#define PCI_VENDOR_ID_MICROSOFT 0x1414
> +#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353
I've never looked at this before, but shouldn't these move to
include/linux/pci_ids.h with all the others? And we've got
another #define of PCI_VENDOR_ID_MICROSOFT in
drivers/net/ethernet/microsoft/mana/gdma_main.c that
could be deleted.
Michael
> +
> /*
> * Common header for Hyper-V ICs
> */
> --
> 2.37.1
Re: [PATCH v4 5/6] dt-bindings: drm/msm/gpu: Add optional resets
On Fri, 19 Aug 2022 22:10:44 +0530, Akhil P Oommen wrote: > Add an optional reference to GPUCC reset which can be used to ensure cx > gdsc collapse during gpu recovery. > > Signed-off-by: Akhil P Oommen > --- > > Changes in v4: > - New patch in v4 > > Documentation/devicetree/bindings/display/msm/gpu.yaml | 7 +++ > 1 file changed, 7 insertions(+) > Acked-by: Rob Herring
Re: [PATCH v2 06/11] dt-bindings: display/msm: move qcom, sc7180-mdss schema to mdss.yaml
On 11/08/2022 11:25, Krzysztof Kozlowski wrote: On 10/07/2022 12:00, Dmitry Baryshkov wrote: Move schema for qcom,sc7180-mdss from dpu-sc7180.yaml to mdss.yaml so that the dpu file describes only the DPU schema. Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dpu-sc7180.yaml | 149 +- .../devicetree/bindings/display/msm/mdss.yaml | 45 +- 2 files changed, 80 insertions(+), 114 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml index d3c3e4b07897..9d4ec0b60c25 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml @@ -10,151 +10,78 @@ maintainers: - Krishna Manikandan description: | - Device tree bindings for MSM Mobile Display Subsystem(MDSS) that encapsulates - sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree - bindings of MDSS and DPU are mentioned for SC7180 target. + Device tree bindings for the DPU display controller for SC7180 target. properties: compatible: items: - - const: qcom,sc7180-mdss + - const: qcom,sc7180-dpu reg: -maxItems: 1 +items: + - description: Address offset and size for mdp register set + - description: Address offset and size for vbif register set reg-names: -const: mdss - - power-domains: -maxItems: 1 +items: + - const: mdp + - const: vbif clocks: items: - - description: Display AHB clock from gcc - - description: Display AHB clock from dispcc + - description: Display hf axi clock + - description: Display ahb clock + - description: Display rotator clock + - description: Display lut clock - description: Display core clock + - description: Display vsync clock clock-names: items: + - const: bus - const: iface - - const: ahb + - const: rot + - const: lut - const: core + - const: vsync interrupts: maxItems: 1 - interrupt-controller: true - - "#address-cells": true - - "#size-cells": true - - "#interrupt-cells": -const: 1 - - iommus: -items: - - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0 - - ranges: true - - interconnects: -items: - - description: Interconnect path specifying the port ids for data bus - - interconnect-names: -const: mdp0-mem + power-domains: +maxItems: 1 - resets: -items: - - description: MDSS_CORE reset + operating-points-v2: true -patternProperties: - "^display-controller@[0-9a-f]+$": -type: object -description: Node containing the properties of DPU. + ports: +$ref: /schemas/graph.yaml#/properties/ports +description: | + Contains the list of output ports from DPU device. These ports + connect to interfaces that are external to the DPU hardware, + such as DSI, DP etc. Each output port contains an endpoint that + describes how it is connected to an external interface. properties: - compatible: -items: - - const: qcom,sc7180-dpu - - reg: -items: - - description: Address offset and size for mdp register set - - description: Address offset and size for vbif register set - - reg-names: -items: - - const: mdp - - const: vbif - - clocks: -items: - - description: Display hf axi clock - - description: Display ahb clock - - description: Display rotator clock - - description: Display lut clock - - description: Display core clock - - description: Display vsync clock - - clock-names: -items: - - const: bus - - const: iface - - const: rot - - const: lut - - const: core - - const: vsync - - interrupts: -maxItems: 1 - - power-domains: -maxItems: 1 - - operating-points-v2: true - - ports: -$ref: /schemas/graph.yaml#/properties/ports -description: | - Contains the list of output ports from DPU device. These ports - connect to interfaces that are external to the DPU hardware, - such as DSI, DP etc. Each output port contains an endpoint that - describes how it is connected to an external interface. - -properties: - port@0: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF1 (DSI1) - - port@2: -$ref: /schemas/graph.yaml#/properties/port -description: DPU_INTF0 (DP) - -required: - - port@0 + port@0: +$ref: /schemas/graph.yaml#/properties/port +description: DPU_INTF1 (DSI1) + + port@2: +$ref: /schemas/graph.yaml#/properties/port
Re: [RESEND v6 6/9] drm: vkms: Refactor the plane composer to accept new formats
On 08/22, Igor Matheus Andrade Torrente wrote: > Hi Melissa, > > On 8/20/22 07:51, Melissa Wen wrote: > > On 08/19, Igor Torrente wrote: > > > Currently the blend function only accepts XRGB_ and ARGB_ > > > as a color input. > > > > > > This patch refactors all the functions related to the plane composition > > > to overcome this limitation. > > > > > > The pixels blend is done using the new internal format. And new handlers > > > are being added to convert a specific format to/from this internal format. > > > > > > So the blend operation depends on these handlers to convert to this common > > > format. The blended result, if necessary, is converted to the writeback > > > buffer format. > > > > > > This patch introduces three major differences to the blend function. > > > 1 - All the planes are blended at once. > > > 2 - The blend calculus is done as per line instead of per pixel. > > > 3 - It is responsible to calculates the CRC and writing the writeback > > > buffer(if necessary). > > > > > > These changes allow us to allocate way less memory in the intermediate > > > buffer to compute these operations. Because now we don't need to > > > have the entire intermediate image lines at once, just one line is > > > enough. > > > > > > | Memory consumption (output dimensions) | > > > |:--:| > > > | Current | This patch| > > > |:--:|:-:| > > > | Width * Heigth | 2 * Width | > > > > > > Beyond memory, we also have a minor performance benefit from all > > > these changes. Results running the IGT[1] test > > > `igt@kms_cursor_crc@pipe-a-cursor-512x512-onscreen` ten times: > > > > > > | Frametime | > > > |:--:| > > > | Implementation | Current | This commit | > > > |:---:|:-:|::| > > > | frametime range | 9~22 ms |5~17 ms | > > > | Average | 11.4 ms |7.8 ms| > > > > > > [1] IGT commit id: bc3f6833a12221a46659535dac06ebb312490eb4 > > > > > > V2: Improves the performance drastically, by performing the operations > > > per-line and not per-pixel(Pekka Paalanen). > > > Minor improvements(Pekka Paalanen). > > > V3: Changes the code to blend the planes all at once. This improves > > > performance, memory consumption, and removes much of the weirdness > > > of the V2(Pekka Paalanen and me). > > > Minor improvements(Pekka Paalanen and me). > > > V4: Rebase the code and adapt it to the new NUM_OVERLAY_PLANES constant. > > > V5: Minor checkpatch fixes and the removal of TO-DO item(Melissa Wen). > > > Several security/robustness improvents(Pekka Paalanen). > > > Removes check_planes_x_bounds function and allows partial > > > partly off-screen(Pekka Paalanen). > > > V6: Fix a mismatch of some variable sizes (Pekka Paalanen). > > > Several minor improvements (Pekka Paalanen). > > > > > > Reported-by: kernel test robot > > > Signed-off-by: Igor Torrente > > > --- > > > Documentation/gpu/vkms.rst| 4 - > > > drivers/gpu/drm/vkms/Makefile | 1 + > > > drivers/gpu/drm/vkms/vkms_composer.c | 320 -- > > > drivers/gpu/drm/vkms/vkms_formats.c | 155 + > > > drivers/gpu/drm/vkms/vkms_formats.h | 12 + > > > drivers/gpu/drm/vkms/vkms_plane.c | 3 + > > > drivers/gpu/drm/vkms/vkms_writeback.c | 3 + > > > 7 files changed, 317 insertions(+), 181 deletions(-) > > > create mode 100644 drivers/gpu/drm/vkms/vkms_formats.c > > > create mode 100644 drivers/gpu/drm/vkms/vkms_formats.h > > > > > > diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst > > > index 973e2d43108b..a49e4ae92653 100644 > > > --- a/Documentation/gpu/vkms.rst > > > +++ b/Documentation/gpu/vkms.rst > > > @@ -118,10 +118,6 @@ Add Plane Features > > > There's lots of plane features we could add support for: > > > -- Clearing primary plane: clear primary plane before plane composition > > > (at the > > > - start) for correctness of pixel blend ops. It also guarantees alpha > > > channel > > > - is cleared in the target buffer for stable crc. [Good to get started] > > > - > > > - ARGB format on primary plane: blend the primary plane into background > > > with > > > translucent alpha. > > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile > > > index 72f779cbfedd..1b28a6a32948 100644 > > > --- a/drivers/gpu/drm/vkms/Makefile > > > +++ b/drivers/gpu/drm/vkms/Makefile > > > @@ -3,6 +3,7 @@ vkms-y := \ > > > vkms_drv.o \ > > > vkms_plane.o \ > > > vkms_output.o \ > > > + vkms_formats.o \ > > > vkms_crtc.o \ > > > vkms_composer.o \ > > > vkms_writeback.o > > > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c > > > b/drivers/gpu/drm/vkms/vkms_composer.c > > > index b9fb408e8973..5b1a8bdd8
Re: [PATCH v2 04/11] dt-bindings: display/msm: split qcom, mdss bindings
On 11/07/2022 14:37, Krzysztof Kozlowski wrote: On 10/07/2022 11:00, Dmitry Baryshkov wrote: Thank you for your patch. There is something to discuss/improve. +required: + - compatible + - reg + - reg-names + - interrupts + - interrupt-controller + - "#interrupt-cells" + - power-domains + - clocks + - clock-names + - "#address-cells" + - "#size-cells" + - ranges + +patternProperties: + "^mdp@(0|[1-9a-f][0-9a-f]*)$": You used some unusual pattern. It's just "[0-9a-f]+" - the device schema's job is not to validate patterns in unit addresses. Another question - why do you allow "@0" alone? I think this was c&p from the other file. Dropped the @0 alternative. +type: object +# TODO: add reference once the mdp5 is converted + + "^dsi@(0|[1-9a-f][0-9a-f]*)$": +$ref: dsi-controller-main.yaml# Best regards, Krzysztof -- With best wishes Dmitry
[PATCH v3 3/3] dt-bindings: msm/dp: handle DP vs eDP difference
The #sound-dai-cells property should be used only for DP controllers. It doesn't make sense for eDP, there is no support for audio output. The aux-bus should not be used for DP controllers. Also p1 MMIO region should be used only for DP controllers. Take care of these differences. Reviewed-by: Rob Herring Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dp-controller.yaml | 26 ++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml index 52cbf00df0ba..f2515af8256f 100644 --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml @@ -24,6 +24,7 @@ properties: - qcom,sm8350-dp reg: +minItems: 4 items: - description: ahb register block - description: aux register block @@ -112,10 +113,33 @@ required: - clock-names - phys - phy-names - - "#sound-dai-cells" - power-domains - ports +allOf: + # AUX BUS does not exist on DP controllers + # Audio output also is present only on DP output + # p1 regions is present on DP, but not on eDP + - if: + properties: +compatible: + contains: +enum: + - qcom,sc7280-edp + - qcom,sc8180x-edp +then: + properties: +"#sound-dai-cells": false +reg: + maxItems: 4 +else: + properties: +aux-bus: false +reg: + minItems: 5 + required: +- "#sound-dai-cells" + additionalProperties: false examples: -- 2.35.1
[PATCH v3 1/3] dt-bindings: msm/dp: mark vdda supplies as deprecated
The commit 85936d4f3815 ("phy: qcom-qmp: add regulator_set_load to dp
phy") moved setting regulator load to the DP PHY driver (QMP). Then, the
commit 7516351bebc1 ("drm/msm/dp: delete vdda regulator related
functions from eDP/DP controller") removed support for VDDA supplies
from the DP controller driver.
Mark these properties as deprecated and drop them from the example.
Acked-by: Rob Herring
Signed-off-by: Dmitry Baryshkov
---
.../devicetree/bindings/display/msm/dp-controller.yaml | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
index 94bc6e1b6451..391910d91e43 100644
--- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
@@ -76,8 +76,10 @@ properties:
"#sound-dai-cells":
const: 0
- vdda-0p9-supply: true
- vdda-1p2-supply: true
+ vdda-0p9-supply:
+deprecated: true
+ vdda-1p2-supply:
+deprecated: true
ports:
$ref: /schemas/graph.yaml#/properties/ports
@@ -140,9 +142,6 @@ examples:
power-domains = <&rpmhpd SC7180_CX>;
-vdda-0p9-supply = <&vdda_usb_ss_dp_core>;
-vdda-1p2-supply = <&vdda_usb_ss_dp_1p2>;
-
ports {
#address-cells = <1>;
#size-cells = <0>;
--
2.35.1
[PATCH v3 0/3] dt-bindings: msm/dp: cleanup Qualcomm DP and eDP bidndings
Fix several issues with the DP and eDP bindings on the Qualcomm platforms. While we are at it, fix several small issues with platform files declaring these controllers. Changes since v2: - Fixed commit message for the patch 1 to mention proper commit IDs. - Dropped dts patches which were picked up by respective tree. Changes since v1: - Reordered patches to cleanup dts first, to remove warnings from DP schema - Split DP register blocks in sc7180.dtsi and sc7280.dtsi - Cleaned up the p1 register block handling: marked it as required for DP and absent for eDP controllers - Dropped unused xo and ref clocks from sc7280-edp node, they belong to eDP PHY. Dmitry Baryshkov (3): dt-bindings: msm/dp: mark vdda supplies as deprecated dt-bindings: msm/dp: add missing properties dt-bindings: msm/dp: handle DP vs eDP difference .../bindings/display/msm/dp-controller.yaml | 47 --- 1 file changed, 41 insertions(+), 6 deletions(-) -- 2.35.1
[PATCH v3 2/3] dt-bindings: msm/dp: add missing properties
Document missing definitions for opp-table (DP controller OPPs), aux-bus (DP AUX BUS) and data-lanes (DP/eDP lanes mapping) properties. Reviewed-by: Stephen Boyd Acked-by: Krzysztof Kozlowski Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dp-controller.yaml | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml index 391910d91e43..52cbf00df0ba 100644 --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml @@ -70,9 +70,21 @@ properties: operating-points-v2: maxItems: 1 + opp-table: true + power-domains: maxItems: 1 + aux-bus: +$ref: /schemas/display/dp-aux-bus.yaml# + + data-lanes: +$ref: /schemas/types.yaml#/definitions/uint32-array +minItems: 1 +maxItems: 4 +items: + maximum: 3 + "#sound-dai-cells": const: 0 -- 2.35.1
[PATCH] drm/msm/iommu: optimize map/unmap
From: Rob Clark
Using map_pages/unmap_pages cuts down on the # of pgtable walks needed
in the process of finding where to insert/remove an entry. The end
result is ~5-10x faster than mapping a single page at a time.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_iommu.c | 91 -
1 file changed, 79 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index a54ed354578b..0f3f60da3314 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -21,6 +21,7 @@ struct msm_iommu_pagetable {
struct msm_mmu base;
struct msm_mmu *parent;
struct io_pgtable_ops *pgtbl_ops;
+ unsigned long pgsize_bitmap;/* Bitmap of page sizes in use */
phys_addr_t ttbr;
u32 asid;
};
@@ -29,23 +30,85 @@ static struct msm_iommu_pagetable *to_pagetable(struct
msm_mmu *mmu)
return container_of(mmu, struct msm_iommu_pagetable, base);
}
+/* based on iommu_pgsize() in iommu.c: */
+static size_t iommu_pgsize(struct msm_iommu_pagetable *pagetable,
+ unsigned long iova, phys_addr_t paddr,
+ size_t size, size_t *count)
+{
+ unsigned int pgsize_idx, pgsize_idx_next;
+ unsigned long pgsizes;
+ size_t offset, pgsize, pgsize_next;
+ unsigned long addr_merge = paddr | iova;
+
+ /* Page sizes supported by the hardware and small enough for @size */
+ pgsizes = pagetable->pgsize_bitmap & GENMASK(__fls(size), 0);
+
+ /* Constrain the page sizes further based on the maximum alignment */
+ if (likely(addr_merge))
+ pgsizes &= GENMASK(__ffs(addr_merge), 0);
+
+ /* Make sure we have at least one suitable page size */
+ BUG_ON(!pgsizes);
+
+ /* Pick the biggest page size remaining */
+ pgsize_idx = __fls(pgsizes);
+ pgsize = BIT(pgsize_idx);
+ if (!count)
+ return pgsize;
+
+ /* Find the next biggest support page size, if it exists */
+ pgsizes = pagetable->pgsize_bitmap & ~GENMASK(pgsize_idx, 0);
+ if (!pgsizes)
+ goto out_set_count;
+
+ pgsize_idx_next = __ffs(pgsizes);
+ pgsize_next = BIT(pgsize_idx_next);
+
+ /*
+* There's no point trying a bigger page size unless the virtual
+* and physical addresses are similarly offset within the larger page.
+*/
+ if ((iova ^ paddr) & (pgsize_next - 1))
+ goto out_set_count;
+
+ /* Calculate the offset to the next page size alignment boundary */
+ offset = pgsize_next - (addr_merge & (pgsize_next - 1));
+
+ /*
+* If size is big enough to accommodate the larger page, reduce
+* the number of smaller pages.
+*/
+ if (offset + pgsize_next <= size)
+ size = offset;
+
+out_set_count:
+ *count = size >> pgsize_idx;
+ return pgsize;
+}
+
static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova,
size_t size)
{
struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
- size_t unmapped = 0;
/* Unmap the block one page at a time */
while (size) {
- unmapped += ops->unmap(ops, iova, 4096, NULL);
- iova += 4096;
- size -= 4096;
+ size_t unmapped, pgsize, count;
+
+ pgsize = iommu_pgsize(pagetable, iova, iova, size, &count);
+
+ unmapped = ops->unmap_pages(ops, iova, pgsize, count, NULL);
+ if (!unmapped)
+ break;
+
+ iova += unmapped;
+ size -= unmapped;
}
iommu_flush_iotlb_all(to_msm_iommu(pagetable->parent)->domain);
- return (unmapped == size) ? 0 : -EINVAL;
+ return (size == 0) ? 0 : -EINVAL;
}
static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova,
@@ -54,7 +117,6 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64
iova,
struct msm_iommu_pagetable *pagetable = to_pagetable(mmu);
struct io_pgtable_ops *ops = pagetable->pgtbl_ops;
struct scatterlist *sg;
- size_t mapped = 0;
u64 addr = iova;
unsigned int i;
@@ -64,15 +126,19 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu,
u64 iova,
/* Map the block one page at a time */
while (size) {
- if (ops->map(ops, addr, phys, 4096, prot, GFP_KERNEL)) {
- msm_iommu_pagetable_unmap(mmu, iova, mapped);
+ size_t pgsize, count, mapped;
+
+ pgsize = iommu_pgsize(pagetable, addr, phys, size,
&count);
+
+ if (ops->map_pages(ops, addr, phys, pgsize, count,
+ prot, GFP_KERNEL, &mapped)) {
+
Re: [PATCH v2 8/9] dt-bindings: msm/dp: add missing properties
On 12/07/2022 02:16, Rob Herring wrote: On Sun, Jul 10, 2022 at 11:41:32AM +0300, Dmitry Baryshkov wrote: Document missing definitions for opp-table (DP controller OPPs), aux-bus (DP AUX BUS) and data-lanes (DP/eDP lanes mapping) properties. Reviewed-by: Stephen Boyd Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/dp-controller.yaml | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml index 391910d91e43..52cbf00df0ba 100644 --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml @@ -70,9 +70,21 @@ properties: operating-points-v2: maxItems: 1 + opp-table: true + power-domains: maxItems: 1 + aux-bus: +$ref: /schemas/display/dp-aux-bus.yaml# + + data-lanes: But this is the wrong location for 'data-lanes'. It belongs in an endpoint node. I rechecked the existing device trees (sc7280-herobrine.dtsi). The data-lanes are already inside the main dp controller node. I'll take a glance on fixing the driver to check the dp_out endpoint (and update existing DT to move the property to the endpoint node), but to make schema compatible with the existing device trees we'd probably still need this property (which can be marked as deprecated once the proper endpoint property is supported). Does that sound plausible? +$ref: /schemas/types.yaml#/definitions/uint32-array +minItems: 1 +maxItems: 4 +items: + maximum: 3 + "#sound-dai-cells": const: 0 -- 2.35.1 -- With best wishes Dmitry
Re: [RESEND v6 2/9] drm: vkms: Rename `vkms_composer` to `vkms_frame_info`
On 08/22, Igor Matheus Andrade Torrente wrote:
> Hi Mellisa,
>
> On 8/20/22 08:00, Melissa Wen wrote:
> > On 08/19, Igor Torrente wrote:
> > > Changes the name of this struct to a more meaningful name.
> > > A name that represents better what this struct is about.
> > >
> > > Composer is the code that do the compositing of the planes.
> > > This struct contains information on the frame used in the output
> > > composition. Thus, vkms_frame_info is a better name to represent
> > > this.
> > >
> > > V5: Fix a commit message typo(Melissa Wen).
> > >
> > > Reviewed-by: Melissa Wen
> > > Signed-off-by: Igor Torrente
> > > ---
> > > drivers/gpu/drm/vkms/vkms_composer.c | 87 ++--
> > > drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
> > > drivers/gpu/drm/vkms/vkms_plane.c| 38 ++--
> > > 3 files changed, 66 insertions(+), 65 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> > > b/drivers/gpu/drm/vkms/vkms_composer.c
> > > index 775b97766e08..0aded4e87e60 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_composer.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> > > @@ -11,11 +11,11 @@
> > > #include "vkms_drv.h"
> > > static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer,
> > > - const struct vkms_composer *composer)
> > > + const struct vkms_frame_info *frame_info)
> > > {
> > > u32 pixel;
> > > - int src_offset = composer->offset + (y * composer->pitch)
> > > - + (x * composer->cpp);
> > > + int src_offset = frame_info->offset + (y * frame_info->pitch)
> > > + + (x * frame_info->cpp);
> > > pixel = *(u32 *)&buffer[src_offset];
> > > @@ -26,24 +26,24 @@ static u32 get_pixel_from_buffer(int x, int y, const
> > > u8 *buffer,
> > >* compute_crc - Compute CRC value on output frame
> > >*
> > >* @vaddr: address to final framebuffer
> > > - * @composer: framebuffer's metadata
> > > + * @frame_info: framebuffer's metadata
> > >*
> > >* returns CRC value computed using crc32 on the visible portion of
> > >* the final framebuffer at vaddr_out
> > >*/
> > > static uint32_t compute_crc(const u8 *vaddr,
> > > - const struct vkms_composer *composer)
> > > + const struct vkms_frame_info *frame_info)
> > > {
> > > int x, y;
> > > u32 crc = 0, pixel = 0;
> > > - int x_src = composer->src.x1 >> 16;
> > > - int y_src = composer->src.y1 >> 16;
> > > - int h_src = drm_rect_height(&composer->src) >> 16;
> > > - int w_src = drm_rect_width(&composer->src) >> 16;
> > > + int x_src = frame_info->src.x1 >> 16;
> > > + int y_src = frame_info->src.y1 >> 16;
> > > + int h_src = drm_rect_height(&frame_info->src) >> 16;
> > > + int w_src = drm_rect_width(&frame_info->src) >> 16;
> > > for (y = y_src; y < y_src + h_src; ++y) {
> > > for (x = x_src; x < x_src + w_src; ++x) {
> > > - pixel = get_pixel_from_buffer(x, y, vaddr, composer);
> > > + pixel = get_pixel_from_buffer(x, y, vaddr, frame_info);
> > > crc = crc32_le(crc, (void *)&pixel,
> > > sizeof(u32));
> > > }
> > > }
> > > @@ -98,8 +98,8 @@ static void x_blend(const u8 *xrgb_src, u8 *xrgb_dst)
> > >* blend - blend value at vaddr_src with value at vaddr_dst
> > >* @vaddr_dst: destination address
> > >* @vaddr_src: source address
> > > - * @dst_composer: destination framebuffer's metadata
> > > - * @src_composer: source framebuffer's metadata
> > > + * @dst_frame_info: destination framebuffer's metadata
> > > + * @src_frame_info: source framebuffer's metadata
> > >* @pixel_blend: blending equation based on plane format
> > >*
> > >* Blend the vaddr_src value with the vaddr_dst value using a pixel
> > > blend
> > > @@ -111,33 +111,33 @@ static void x_blend(const u8 *xrgb_src, u8
> > > *xrgb_dst)
> > >* pixel color values
> > >*/
> > > static void blend(void *vaddr_dst, void *vaddr_src,
> > > - struct vkms_composer *dst_composer,
> > > - struct vkms_composer *src_composer,
> > > + struct vkms_frame_info *dst_frame_info,
> > > + struct vkms_frame_info *src_frame_info,
> > > void (*pixel_blend)(const u8 *, u8 *))
> > > {
> > > int i, j, j_dst, i_dst;
> > > int offset_src, offset_dst;
> > > u8 *pixel_dst, *pixel_src;
> > > - int x_src = src_composer->src.x1 >> 16;
> > > - int y_src = src_composer->src.y1 >> 16;
> > > + int x_src = src_frame_info->src.x1 >> 16;
> > > + int y_src = src_frame_info->src.y1 >> 16;
> > > - int x_dst = src_composer->dst.x1;
> > > - int y_dst = src_composer->dst.y1;
> > > - int h_dst = drm_rect_height(&src_composer->dst);
> > > - int w_dst = drm_rect_width(&src_composer->dst);
> > > + int x_dst = src_frame_info->dst.x
Re: [PATCH] drm/msm/dp: add atomic_check to bridge ops
On 22/08/2022 20:32, Abhinav Kumar wrote: On 8/22/2022 9:49 AM, Dmitry Baryshkov wrote: On 22/08/2022 19:38, Abhinav Kumar wrote: Hi Dmitry On 8/22/2022 9:18 AM, Dmitry Baryshkov wrote: On 17/08/2022 21:01, Kuogee Hsieh wrote: DRM commit_tails() will disable downstream crtc/encoder/bridge if both disable crtc is required and crtc->active is set before pushing a new frame downstream. There is a rare case that user space display manager issue an extra screen update immediately followed by close DRM device while down stream display interface is disabled. This extra screen update will timeout due to the downstream interface is disabled but will cause crtc->active be set. Hence the followed commit_tails() called by drm_release() will pass the disable downstream crtc/encoder/bridge conditions checking even downstream interface is disabled. This cause the crash to happen at dp_bridge_disable() due to it trying to access the main link register to push the idle pattern out while main link clocks is disabled. This patch adds atomic_check to prevent the extra frame will not be pushed down if display interface is down so that crtc->active will not be set neither. This will fail the conditions checking of disabling down stream crtc/encoder/bridge which prevent drm_release() from calling dp_bridge_disable() so that crash at dp_bridge_disable() prevented. I must admit I had troubles parsing this description. However if I got you right, I think the check that the main link clock is running in the dp_bridge_disable() or dp_ctrl_push_idle() would be a better fix. Originally, thats what was posted https://patchwork.freedesktop.org/patch/496984/. This patch is also not so correct from my POV. It checks for the hpd status, while in reality it should check for main link clocks being enabled. We can push another fix to check for the clk state instead of the hpd status. But I must say we are again just masking something which the fwk should have avoided isnt it? As per the doc in the include/drm/drm_bridge.h it says, "* * The bridge can assume that the display pipe (i.e. clocks and timing * signals) feeding it is still running when this callback is called. *" Yes, that's what I meant about this chunk begging to go to the core. In my opinion, if we are talking about the disconnected sinks, it is the framework who should disallow submitting the frames to the disconnected sinks. By adding an extra layers of protection in the driver, we are just avoiding another issue but the commit should not have been issued in the first place. So shouldnt we do both then? That is add protection to check if clock is ON and also, reject commits when display is disconnected. Then it seemed like we were just protecting against an issue in the framework which was allowing the frames to be pushed even after the display was disconnected. The DP driver did send out the disconnect event correctly and as per the logs, this frame came down after that and the DRM fwk did allow it. So after discussing on IRC with Rob, we came up with this approach that if the display is not connected, then atomic_check should fail. That way the commit will not happen. Just seemed a bit cleaner instead of adding all our protections. The check to fail atomic_check if display is not connected seems out of place. In its current way it begs go to the upper layer, forbidding using disconnected sinks for all the drivers. There is nothing special in the MSM DP driver with respect to the HPD events processing and failing atomic_check() based on that. Why all the drivers? This is only for MSM DP bridge. Yes, we change the MSM DRM driver. But the check is generic enough. I'm not actually insisting on pushing the check to the core, just trying to understand the real cause here. SError Interrupt on CPU7, code 0xbe000411 -- SError CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19 Hardware name: Google Lazor (rev3 - 8) (DT) pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : __cmpxchg_case_acq_32+0x14/0x2c lr : do_raw_spin_lock+0xa4/0xdc sp : ffc01092b6a0 x29: ffc01092b6a0 x28: 0028 x27: 0038 x26: 0004 x25: ffd2973dce48 x24: x23: x22: x21: ffd2978d0008 x20: ffd2978d0008 x19: ff80ff759fc0 x18: x17: 004800a501260460 x16: 0441043b04600438 x15: 0438089807d0 x14: 07b0089807800780 x13: x12: x11: 0438 x10: 07d0 x9 : ffd2973e09e4 x8 : ff8092d53300 x7 : ff808902e8b8 x6 : 0001 x5 : ff808902e880 x4 : x3 : ff80ff759fc0 x2 : 0001 x1 : x0 : ff80ff759fc0 Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19 Hardware name: Google Lazor (rev3 - 8) (DT) Call
Re: [RESEND v6 2/9] drm: vkms: Rename `vkms_composer` to `vkms_frame_info`
Hi Mellisa,
On 8/20/22 08:00, Melissa Wen wrote:
On 08/19, Igor Torrente wrote:
Changes the name of this struct to a more meaningful name.
A name that represents better what this struct is about.
Composer is the code that do the compositing of the planes.
This struct contains information on the frame used in the output
composition. Thus, vkms_frame_info is a better name to represent
this.
V5: Fix a commit message typo(Melissa Wen).
Reviewed-by: Melissa Wen
Signed-off-by: Igor Torrente
---
drivers/gpu/drm/vkms/vkms_composer.c | 87 ++--
drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
drivers/gpu/drm/vkms/vkms_plane.c| 38 ++--
3 files changed, 66 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 775b97766e08..0aded4e87e60 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -11,11 +11,11 @@
#include "vkms_drv.h"
static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer,
-const struct vkms_composer *composer)
+const struct vkms_frame_info *frame_info)
{
u32 pixel;
- int src_offset = composer->offset + (y * composer->pitch)
- + (x * composer->cpp);
+ int src_offset = frame_info->offset + (y * frame_info->pitch)
+ + (x * frame_info->cpp);
pixel = *(u32 *)&buffer[src_offset];
@@ -26,24 +26,24 @@ static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer,
* compute_crc - Compute CRC value on output frame
*
* @vaddr: address to final framebuffer
- * @composer: framebuffer's metadata
+ * @frame_info: framebuffer's metadata
*
* returns CRC value computed using crc32 on the visible portion of
* the final framebuffer at vaddr_out
*/
static uint32_t compute_crc(const u8 *vaddr,
- const struct vkms_composer *composer)
+ const struct vkms_frame_info *frame_info)
{
int x, y;
u32 crc = 0, pixel = 0;
- int x_src = composer->src.x1 >> 16;
- int y_src = composer->src.y1 >> 16;
- int h_src = drm_rect_height(&composer->src) >> 16;
- int w_src = drm_rect_width(&composer->src) >> 16;
+ int x_src = frame_info->src.x1 >> 16;
+ int y_src = frame_info->src.y1 >> 16;
+ int h_src = drm_rect_height(&frame_info->src) >> 16;
+ int w_src = drm_rect_width(&frame_info->src) >> 16;
for (y = y_src; y < y_src + h_src; ++y) {
for (x = x_src; x < x_src + w_src; ++x) {
- pixel = get_pixel_from_buffer(x, y, vaddr, composer);
+ pixel = get_pixel_from_buffer(x, y, vaddr, frame_info);
crc = crc32_le(crc, (void *)&pixel, sizeof(u32));
}
}
@@ -98,8 +98,8 @@ static void x_blend(const u8 *xrgb_src, u8 *xrgb_dst)
* blend - blend value at vaddr_src with value at vaddr_dst
* @vaddr_dst: destination address
* @vaddr_src: source address
- * @dst_composer: destination framebuffer's metadata
- * @src_composer: source framebuffer's metadata
+ * @dst_frame_info: destination framebuffer's metadata
+ * @src_frame_info: source framebuffer's metadata
* @pixel_blend: blending equation based on plane format
*
* Blend the vaddr_src value with the vaddr_dst value using a pixel blend
@@ -111,33 +111,33 @@ static void x_blend(const u8 *xrgb_src, u8 *xrgb_dst)
* pixel color values
*/
static void blend(void *vaddr_dst, void *vaddr_src,
- struct vkms_composer *dst_composer,
- struct vkms_composer *src_composer,
+ struct vkms_frame_info *dst_frame_info,
+ struct vkms_frame_info *src_frame_info,
void (*pixel_blend)(const u8 *, u8 *))
{
int i, j, j_dst, i_dst;
int offset_src, offset_dst;
u8 *pixel_dst, *pixel_src;
- int x_src = src_composer->src.x1 >> 16;
- int y_src = src_composer->src.y1 >> 16;
+ int x_src = src_frame_info->src.x1 >> 16;
+ int y_src = src_frame_info->src.y1 >> 16;
- int x_dst = src_composer->dst.x1;
- int y_dst = src_composer->dst.y1;
- int h_dst = drm_rect_height(&src_composer->dst);
- int w_dst = drm_rect_width(&src_composer->dst);
+ int x_dst = src_frame_info->dst.x1;
+ int y_dst = src_frame_info->dst.y1;
+ int h_dst = drm_rect_height(&src_frame_info->dst);
+ int w_dst = drm_rect_width(&src_frame_info->dst);
int y_limit = y_src + h_dst;
int x_limit = x_src + w_dst;
for (i = y_src, i_dst = y_dst; i < y_limit; ++i) {
for (j = x_src, j_dst = x_dst; j < x_limit; ++j) {
- offset_dst = dst_composer->offset
-+ (i_dst * dst_composer->pitch)
-
Re: [RESEND v6 6/9] drm: vkms: Refactor the plane composer to accept new formats
Hi Melissa,
On 8/20/22 07:51, Melissa Wen wrote:
On 08/19, Igor Torrente wrote:
Currently the blend function only accepts XRGB_ and ARGB_
as a color input.
This patch refactors all the functions related to the plane composition
to overcome this limitation.
The pixels blend is done using the new internal format. And new handlers
are being added to convert a specific format to/from this internal format.
So the blend operation depends on these handlers to convert to this common
format. The blended result, if necessary, is converted to the writeback
buffer format.
This patch introduces three major differences to the blend function.
1 - All the planes are blended at once.
2 - The blend calculus is done as per line instead of per pixel.
3 - It is responsible to calculates the CRC and writing the writeback
buffer(if necessary).
These changes allow us to allocate way less memory in the intermediate
buffer to compute these operations. Because now we don't need to
have the entire intermediate image lines at once, just one line is
enough.
| Memory consumption (output dimensions) |
|:--:|
| Current | This patch|
|:--:|:-:|
| Width * Heigth | 2 * Width |
Beyond memory, we also have a minor performance benefit from all
these changes. Results running the IGT[1] test
`igt@kms_cursor_crc@pipe-a-cursor-512x512-onscreen` ten times:
| Frametime |
|:--:|
| Implementation | Current | This commit |
|:---:|:-:|::|
| frametime range | 9~22 ms |5~17 ms |
| Average | 11.4 ms |7.8 ms|
[1] IGT commit id: bc3f6833a12221a46659535dac06ebb312490eb4
V2: Improves the performance drastically, by performing the operations
per-line and not per-pixel(Pekka Paalanen).
Minor improvements(Pekka Paalanen).
V3: Changes the code to blend the planes all at once. This improves
performance, memory consumption, and removes much of the weirdness
of the V2(Pekka Paalanen and me).
Minor improvements(Pekka Paalanen and me).
V4: Rebase the code and adapt it to the new NUM_OVERLAY_PLANES constant.
V5: Minor checkpatch fixes and the removal of TO-DO item(Melissa Wen).
Several security/robustness improvents(Pekka Paalanen).
Removes check_planes_x_bounds function and allows partial
partly off-screen(Pekka Paalanen).
V6: Fix a mismatch of some variable sizes (Pekka Paalanen).
Several minor improvements (Pekka Paalanen).
Reported-by: kernel test robot
Signed-off-by: Igor Torrente
---
Documentation/gpu/vkms.rst| 4 -
drivers/gpu/drm/vkms/Makefile | 1 +
drivers/gpu/drm/vkms/vkms_composer.c | 320 --
drivers/gpu/drm/vkms/vkms_formats.c | 155 +
drivers/gpu/drm/vkms/vkms_formats.h | 12 +
drivers/gpu/drm/vkms/vkms_plane.c | 3 +
drivers/gpu/drm/vkms/vkms_writeback.c | 3 +
7 files changed, 317 insertions(+), 181 deletions(-)
create mode 100644 drivers/gpu/drm/vkms/vkms_formats.c
create mode 100644 drivers/gpu/drm/vkms/vkms_formats.h
diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 973e2d43108b..a49e4ae92653 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -118,10 +118,6 @@ Add Plane Features
There's lots of plane features we could add support for:
-- Clearing primary plane: clear primary plane before plane composition (at the
- start) for correctness of pixel blend ops. It also guarantees alpha channel
- is cleared in the target buffer for stable crc. [Good to get started]
-
- ARGB format on primary plane: blend the primary plane into background with
translucent alpha.
diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
index 72f779cbfedd..1b28a6a32948 100644
--- a/drivers/gpu/drm/vkms/Makefile
+++ b/drivers/gpu/drm/vkms/Makefile
@@ -3,6 +3,7 @@ vkms-y := \
vkms_drv.o \
vkms_plane.o \
vkms_output.o \
+ vkms_formats.o \
vkms_crtc.o \
vkms_composer.o \
vkms_writeback.o
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index b9fb408e8973..5b1a8bdd8268 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -7,204 +7,188 @@
#include
#include
#include
+#include
#include "vkms_drv.h"
-static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer,
-const struct vkms_frame_info *frame_info)
+static u16 pre_mul_blend_channel(u16 src, u16 dst, u16 alpha)
{
- u32 pixel;
- int src_offset = frame_info->offset + (y * frame_info->pitch)
- + (x * frame_info->cpp);
+ u32 new_color;
- pixel = *(u32 *)&buffer[src_offset];
+ new_color = (src * 0x + d
Re: [PATCH v2] dt-bindings: gpu: arm,mali: restrict opp-table to objects
On Thu, 18 Aug 2022 09:17:13 +0300, Krzysztof Kozlowski wrote: > Simple 'opp-table:true' accepts a boolean property as opp-table, so > restrict it to object to properly enforce real OPP table nodes. > > Signed-off-by: Krzysztof Kozlowski > > --- > > Changes since v1: > 1. Correct typo in msg. > --- > Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml | 3 ++- > Documentation/devicetree/bindings/gpu/arm,mali-utgard.yaml | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > Applied, thanks!
Re: [PATCH 3/4] dt-bindings: display/msm/gmu: account for different GMU variants
On 06/07/2022 18:52, Krzysztof Kozlowski wrote: On 06/07/2022 16:52, Dmitry Baryshkov wrote: Make display/msm/gmu.yaml describe all existing GMU variants rather than just the 630.2 (SDM845) version of it. Signed-off-by: Dmitry Baryshkov --- .../devicetree/bindings/display/msm/gmu.yaml | 166 +++--- 1 file changed, 146 insertions(+), 20 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml b/Documentation/devicetree/bindings/display/msm/gmu.yaml index fe55611d2603..67fdeeabae0c 100644 --- a/Documentation/devicetree/bindings/display/msm/gmu.yaml +++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml @@ -20,35 +20,24 @@ description: | properties: compatible: items: - - enum: - - qcom,adreno-gmu-630.2 + - pattern: '^qcom,adreno-gmu-6[0-9][0-9]\.[0-9]$' - const: qcom,adreno-gmu reg: -items: - - description: Core GMU registers - - description: GMU PDC registers - - description: GMU PDC sequence registers +minItems: 3 +maxItems: 4 reg-names: -items: - - const: gmu - - const: gmu_pdc - - const: gmu_pdc_seq +minItems: 3 +maxItems: 4 clocks: -items: - - description: GMU clock - - description: GPU CX clock - - description: GPU AXI clock - - description: GPU MEMNOC clock +minItems: 4 +maxItems: 7 clock-names: -items: - - const: gmu - - const: cxo - - const: axi - - const: memnoc +minItems: 4 +maxItems: 7 interrupts: items: @@ -76,6 +65,9 @@ properties: operating-points-v2: true + opp-table: +type: object instead: opp-table:true Wouldn't this allow e.g. using just 'opp-table;' as a flag? + Best regards, Krzysztof -- With best wishes Dmitry
Re: [PATCH] drm/panel-edp: add AUO B133UAN02.1 panel entry
Hi, On Mon, Aug 22, 2022 at 10:33 AM Doug Anderson wrote: > > Hi, > > On Mon, Aug 22, 2022 at 6:35 AM Johan Hovold wrote: > > > > On Fri, Jul 22, 2022 at 11:48:40AM +0200, Johan Hovold wrote: > > > On Mon, Jul 11, 2022 at 09:52:02AM +0200, Johan Hovold wrote: > > > > Add an eDP panel entry for AUO B133UAN02.1. > > > > > > > > Due to lack of documentation, use the delay_200_500_e50 timings like > > > > some other AUO entries for now. > > > > > > > > Signed-off-by: Johan Hovold > > > > > > Any comments to this one? > > > > > > It looks like it hasn't shown up in the dri-devel patchwork and just > > > want to make sure it isn't lost. > > > > > > Rob Clark mentioned something about a spam filter on IRC. > > > > This one still hasn't been picked up. > > > > Is this something you can do, Doug? I noticed you applied a couple of > > patches to this driver recently. > > > > Or who is really responsible for this driver? > > Officially it falls within this section of maintainers: > > DRM PANEL DRIVERS > M: Thierry Reding > R: Sam Ravnborg > L: dri-devel@lists.freedesktop.org > S: Maintained > T: git git://anongit.freedesktop.org/drm/drm-misc > F: Documentation/devicetree/bindings/display/panel/ > F: drivers/gpu/drm/drm_panel.c > F: drivers/gpu/drm/panel/ > F: include/drm/drm_panel.h > > ...and then you just have to know that if the "tree" is drm-misc that > it falls under drm-misc rules. That means that anyone who is a > drm-misc-next committer can commit it. > > I've been trying to keep an eye on panel-edp ever since I split it out > from panel-simple, though. I'll post up a MAINTAINERS entry to try to > make that more official. Posted a patch to add myself in MAINTAINERS: https://lore.kernel.org/r/20220822105340.1.I66a9a5577f9b0af66492ef13c47bc78ed85e5d6b@changeid ...though I had a brain-fail and didn't CC anyone on it. ;-) -Doug
[PATCH] MAINTAINERS: Add myself as a reviewer for panel-edp.c
panel-edp changes go through the drm-misc tree (as per the "DRM PANEL DRIVERS" entry in MAINTAINERS), but ever since splitting panel-edp out of panel-simple I've been trying to keep a close eye on it. Make that official by listing me as a reviewer. Signed-off-by: Douglas Anderson --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0f9366144d31..fc62434f693f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6343,6 +6343,11 @@ S: Maintained F: Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.yaml F: drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c +DRM DRIVER FOR GENERIC EDP PANELS +R: Douglas Anderson +F: Documentation/devicetree/bindings/display/panel/panel-edp.yaml +F: drivers/gpu/drm/panel/panel-edp.c + DRM DRIVER FOR GENERIC USB DISPLAY M: Noralf Trønnes S: Maintained -- 2.37.2.609.g9ff673ca1a-goog
Re: [PATCH] drm/msm/dpu: drop unused memory allocation
On 8/22/2022 10:24 AM, Dmitry Baryshkov wrote:
Drop the dpu_cfg variable and corresponding kzalloc, which became unused
after changing hw catalog to static configuration.
Fixes: de7d480f5e8c ("drm/msm/dpu: make dpu hardware catalog static const")
Reported-by: kernel test robot
Reported-by: Dan Carpenter
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 0239a811d5ec..50ab17c9afd2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -1939,11 +1939,6 @@ static const struct dpu_mdss_hw_cfg_handler
cfg_handler[] = {
const struct dpu_mdss_cfg *dpu_hw_catalog_init(u32 hw_rev)
{
int i;
- struct dpu_mdss_cfg *dpu_cfg;
-
- dpu_cfg = kzalloc(sizeof(*dpu_cfg), GFP_KERNEL);
- if (!dpu_cfg)
- return ERR_PTR(-ENOMEM);
for (i = 0; i < ARRAY_SIZE(cfg_handler); i++) {
if (cfg_handler[i].hw_rev == hw_rev)
Re: [Freedreno] [PATCH v2.5] drm/msm/dsi: switch to DRM_PANEL_BRIDGE
On 15/07/2022 00:54, Abhinav Kumar wrote:
On 7/12/2022 6:22 AM, Dmitry Baryshkov wrote:
Currently the DSI driver has two separate paths: one if the next device
in a chain is a bridge and another one if the panel is connected
directly to the DSI host. Simplify the code path by using panel-bridge
driver (already selected in Kconfig) and dropping support for
handling the panel directly.
Signed-off-by: Dmitry Baryshkov
---
I'm not sending this as a separate patchset (I'd like to sort out mdp5
first), but more of a preview of changes related to
msm_dsi_manager_ext_bridge_init().
---
drivers/gpu/drm/msm/dsi/dsi.c | 35 +---
drivers/gpu/drm/msm/dsi/dsi.h | 16 +-
drivers/gpu/drm/msm/dsi/dsi_host.c | 25 ---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 283 +++---
4 files changed, 36 insertions(+), 323 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c
b/drivers/gpu/drm/msm/dsi/dsi.c
index 1625328fa430..4edb9167e600 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -6,14 +6,6 @@
#include "dsi.h"
#include "dsi_cfg.h"
-struct drm_encoder *msm_dsi_get_encoder(struct msm_dsi *msm_dsi)
-{
- if (!msm_dsi || !msm_dsi_device_connected(msm_dsi))
- return NULL;
-
- return msm_dsi->encoder;
-}
-
bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
{
unsigned long host_flags =
msm_dsi_host_get_mode_flags(msm_dsi->host);
@@ -220,7 +212,6 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi,
struct drm_device *dev,
struct drm_encoder *encoder)
{
struct msm_drm_private *priv;
- struct drm_bridge *ext_bridge;
int ret;
if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
@@ -254,26 +245,10 @@ int msm_dsi_modeset_init(struct msm_dsi
*msm_dsi, struct drm_device *dev,
goto fail;
}
- /*
- * check if the dsi encoder output is connected to a panel or an
- * external bridge. We create a connector only if we're connected
to a
- * drm_panel device. When we're connected to an external bridge, we
- * assume that the drm_bridge driver will create the connector
itself.
- */
- ext_bridge = msm_dsi_host_get_bridge(msm_dsi->host);
-
- if (ext_bridge)
- msm_dsi->connector =
- msm_dsi_manager_ext_bridge_init(msm_dsi->id);
- else
- msm_dsi->connector =
- msm_dsi_manager_connector_init(msm_dsi->id);
-
- if (IS_ERR(msm_dsi->connector)) {
- ret = PTR_ERR(msm_dsi->connector);
+ ret = msm_dsi_manager_ext_bridge_init(msm_dsi->id);
+ if (ret) {
DRM_DEV_ERROR(dev->dev,
"failed to create dsi connector: %d\n", ret);
- msm_dsi->connector = NULL;
goto fail;
}
@@ -287,12 +262,6 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi,
struct drm_device *dev,
msm_dsi->bridge = NULL;
}
- /* don't destroy connector if we didn't make it */
- if (msm_dsi->connector && !msm_dsi->external_bridge)
- msm_dsi->connector->funcs->destroy(msm_dsi->connector);
-
- msm_dsi->connector = NULL;
From what i can see all the usages of msm_dsi->connector are removed
after this change. So can we drop that?
The connector field is dropped from the msm_dsi struct. If you are
asking about the msm_dsi_modeset_init(), we can not drop it since we
require the DRM device with GEM being initialized in order to allocate
DSI DMA buffer. We can think about moving DMA buffer allocation towards
the usage point, however this is definitely a separate commit.
[skipped]
*msm_dsi_manager_ext_bridge_init(u8 id)
ret = drm_bridge_attach(encoder, ext_bridge, int_bridge,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret == -EINVAL) {
- struct drm_connector *connector;
- struct list_head *connector_list;
-
- /* link the internal dsi bridge to the external bridge */
- drm_bridge_attach(encoder, ext_bridge, int_bridge, 0);
-
/*
- * we need the drm_connector created by the external bridge
- * driver (or someone else) to feed it to our driver's
- * priv->connector[] list, mainly for msm_fbdev_init()
+ * link the internal dsi bridge to the external bridge,
+ * connector is created by the next bridge.
*/
- connector_list = &dev->mode_config.connector_list;
+ ret = drm_bridge_attach(encoder, ext_bridge, int_bridge, 0);
+ if (ret < 0)
+ return ret;
+ } else {
+ struct drm_connector *connector;
- list_for_each_entry(connector, connector_list, head) {
- if (drm_connector_has_possible_encoder(connector, encoder))
- return connector;
+ /* We are in charge of the connector, create one now. */
+ connector = drm_bridge_connector_init(dev, encoder);
+ if (IS_ERR(connector)) {
+ DRM_ERROR("Unable to create bridge connector\n");
+ return PTR_ERR(connector
[PATCH] drm/msm/dsi: drop the hpd worker
It makes no sense to have the HPD worker in the MSM DSI driver anymore.
It is only queued from the dsi_host_attach/detach() callbacks, where
it plays no useful role. Either way the panel or next bridge will be
present and will report it's status directly.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index a34078497af1..43bf84e92a7c 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -144,7 +144,6 @@ struct msm_dsi_host {
u32 err_work_state;
struct work_struct err_work;
- struct work_struct hpd_work;
struct workqueue_struct *workqueue;
/* DSI 6G TX buffer*/
@@ -1500,14 +1499,6 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
return len;
}
-static void dsi_hpd_worker(struct work_struct *work)
-{
- struct msm_dsi_host *msm_host =
- container_of(work, struct msm_dsi_host, hpd_work);
-
- drm_helper_hpd_irq_event(msm_host->dev);
-}
-
static void dsi_err_worker(struct work_struct *work)
{
struct msm_dsi_host *msm_host =
@@ -1697,8 +1688,6 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
return ret;
DBG("id=%d", msm_host->id);
- if (msm_host->dev)
- queue_work(msm_host->workqueue, &msm_host->hpd_work);
return 0;
}
@@ -1713,8 +1702,6 @@ static int dsi_host_detach(struct mipi_dsi_host *host,
msm_host->device_node = NULL;
DBG("id=%d", msm_host->id);
- if (msm_host->dev)
- queue_work(msm_host->workqueue, &msm_host->hpd_work);
return 0;
}
@@ -2126,7 +2113,6 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
/* setup workqueue */
msm_host->workqueue = alloc_ordered_workqueue("dsi_drm_work", 0);
INIT_WORK(&msm_host->err_work, dsi_err_worker);
- INIT_WORK(&msm_host->hpd_work, dsi_hpd_worker);
msm_dsi->id = msm_host->id;
--
2.35.1
Re: [PATCH] drm/msm/dp: add atomic_check to bridge ops
On 8/22/2022 9:49 AM, Dmitry Baryshkov wrote: On 22/08/2022 19:38, Abhinav Kumar wrote: Hi Dmitry On 8/22/2022 9:18 AM, Dmitry Baryshkov wrote: On 17/08/2022 21:01, Kuogee Hsieh wrote: DRM commit_tails() will disable downstream crtc/encoder/bridge if both disable crtc is required and crtc->active is set before pushing a new frame downstream. There is a rare case that user space display manager issue an extra screen update immediately followed by close DRM device while down stream display interface is disabled. This extra screen update will timeout due to the downstream interface is disabled but will cause crtc->active be set. Hence the followed commit_tails() called by drm_release() will pass the disable downstream crtc/encoder/bridge conditions checking even downstream interface is disabled. This cause the crash to happen at dp_bridge_disable() due to it trying to access the main link register to push the idle pattern out while main link clocks is disabled. This patch adds atomic_check to prevent the extra frame will not be pushed down if display interface is down so that crtc->active will not be set neither. This will fail the conditions checking of disabling down stream crtc/encoder/bridge which prevent drm_release() from calling dp_bridge_disable() so that crash at dp_bridge_disable() prevented. I must admit I had troubles parsing this description. However if I got you right, I think the check that the main link clock is running in the dp_bridge_disable() or dp_ctrl_push_idle() would be a better fix. Originally, thats what was posted https://patchwork.freedesktop.org/patch/496984/. This patch is also not so correct from my POV. It checks for the hpd status, while in reality it should check for main link clocks being enabled. We can push another fix to check for the clk state instead of the hpd status. But I must say we are again just masking something which the fwk should have avoided isnt it? As per the doc in the include/drm/drm_bridge.h it says, "* * The bridge can assume that the display pipe (i.e. clocks and timing * signals) feeding it is still running when this callback is called. *" By adding an extra layers of protection in the driver, we are just avoiding another issue but the commit should not have been issued in the first place. So shouldnt we do both then? That is add protection to check if clock is ON and also, reject commits when display is disconnected. Then it seemed like we were just protecting against an issue in the framework which was allowing the frames to be pushed even after the display was disconnected. The DP driver did send out the disconnect event correctly and as per the logs, this frame came down after that and the DRM fwk did allow it. So after discussing on IRC with Rob, we came up with this approach that if the display is not connected, then atomic_check should fail. That way the commit will not happen. Just seemed a bit cleaner instead of adding all our protections. The check to fail atomic_check if display is not connected seems out of place. In its current way it begs go to the upper layer, forbidding using disconnected sinks for all the drivers. There is nothing special in the MSM DP driver with respect to the HPD events processing and failing atomic_check() based on that. Why all the drivers? This is only for MSM DP bridge. SError Interrupt on CPU7, code 0xbe000411 -- SError CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19 Hardware name: Google Lazor (rev3 - 8) (DT) pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : __cmpxchg_case_acq_32+0x14/0x2c lr : do_raw_spin_lock+0xa4/0xdc sp : ffc01092b6a0 x29: ffc01092b6a0 x28: 0028 x27: 0038 x26: 0004 x25: ffd2973dce48 x24: x23: x22: x21: ffd2978d0008 x20: ffd2978d0008 x19: ff80ff759fc0 x18: x17: 004800a501260460 x16: 0441043b04600438 x15: 0438089807d0 x14: 07b0089807800780 x13: x12: x11: 0438 x10: 07d0 x9 : ffd2973e09e4 x8 : ff8092d53300 x7 : ff808902e8b8 x6 : 0001 x5 : ff808902e880 x4 : x3 : ff80ff759fc0 x2 : 0001 x1 : x0 : ff80ff759fc0 Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19 Hardware name: Google Lazor (rev3 - 8) (DT) Call trace: dump_backtrace.part.0+0xbc/0xe4 show_stack+0x24/0x70 dump_stack_lvl+0x68/0x84 dump_stack+0x18/0x34 panic+0x14c/0x32c nmi_panic+0x58/0x7c arm64_serror_panic+0x78/0x84 do_serror+0x40/0x64 el1h_64_error_handler+0x30/0x48 el1h_64_error+0x68/0x6c __cmpxchg_case_acq_32+0x14/0x2c _raw_spin_lock_irqsave+0x38/0x4c lock_timer_base+0x40/0x78 __mod_timer+0xf4/0x25c schedule_timeout+0xd4/0xfc __wait_for_common+0xac/0x140 wait
Re: [PATCH] drm/panel-edp: add AUO B133UAN02.1 panel entry
Hi, On Mon, Aug 22, 2022 at 6:35 AM Johan Hovold wrote: > > On Fri, Jul 22, 2022 at 11:48:40AM +0200, Johan Hovold wrote: > > On Mon, Jul 11, 2022 at 09:52:02AM +0200, Johan Hovold wrote: > > > Add an eDP panel entry for AUO B133UAN02.1. > > > > > > Due to lack of documentation, use the delay_200_500_e50 timings like > > > some other AUO entries for now. > > > > > > Signed-off-by: Johan Hovold > > > > Any comments to this one? > > > > It looks like it hasn't shown up in the dri-devel patchwork and just > > want to make sure it isn't lost. > > > > Rob Clark mentioned something about a spam filter on IRC. > > This one still hasn't been picked up. > > Is this something you can do, Doug? I noticed you applied a couple of > patches to this driver recently. > > Or who is really responsible for this driver? Officially it falls within this section of maintainers: DRM PANEL DRIVERS M: Thierry Reding R: Sam Ravnborg L: dri-devel@lists.freedesktop.org S: Maintained T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/devicetree/bindings/display/panel/ F: drivers/gpu/drm/drm_panel.c F: drivers/gpu/drm/panel/ F: include/drm/drm_panel.h ...and then you just have to know that if the "tree" is drm-misc that it falls under drm-misc rules. That means that anyone who is a drm-misc-next committer can commit it. I've been trying to keep an eye on panel-edp ever since I split it out from panel-simple, though. I'll post up a MAINTAINERS entry to try to make that more official. In any case, I've landed this on drm-misc-next: ee50b0024408 drm/panel-edp: add AUO B133UAN02.1 panel entry -Doug
Re: [PATCH v6 4/4] drm/i915/fbdev: do not create fbdev if HPD is suspended
On Fri, Jul 22, 2022 at 02:51:43PM +0200, Andrzej Hajda wrote:
> In case of deferred FB setup core can try to create new
> framebuffer. Disallow it if hpd_suspended flag is set.
>
> Signed-off-by: Andrzej Hajda
Reviewed-by: Imre Deak
> ---
> drivers/gpu/drm/i915/display/intel_fbdev.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 94ddc0f34fde64..fb8dbd532b9e05 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -210,6 +210,12 @@ static int intelfb_create(struct drm_fb_helper *helper,
> struct drm_i915_gem_object *obj;
> int ret;
>
> + mutex_lock(&ifbdev->hpd_lock);
> + ret = ifbdev->hpd_suspended ? -EAGAIN : 0;
> + mutex_unlock(&ifbdev->hpd_lock);
> + if (ret)
> + return ret;
> +
> if (intel_fb &&
> (sizes->fb_width > intel_fb->base.width ||
>sizes->fb_height > intel_fb->base.height)) {
> --
> 2.25.1
>
Re: [Intel-gfx] [PATCH v6 3/4] drm/i915/display: add hotplug.suspended flag
On Fri, Jul 22, 2022 at 02:51:42PM +0200, Andrzej Hajda wrote:
> HPD events during driver removal can be generated by hardware and
> software frameworks - drm_dp_mst, the former we can avoid by disabling
> interrupts, the latter can be triggered by any drm_dp_mst transaction,
> and this is too late. Introducing suspended flag allows to solve this
> chicken-egg problem.
intel_hpd_cancel_work() is always called after suspending MST and
disabling IRQs (with the order I suggested in patch 1). If both of these
have disabled the corresponding functionality (MST, IRQs) properly with
all their MST/IRQ scheduled works guaranteed to not get rescheduled,
then it's not clear how could either intel_hpd_trigger_irq() or an IRQ
work run. So the problematic sequence would need a better explanation.
There's also already
dev_priv->runtime_pm.irqs_enabled
showing if hotplug interrupts are disabled (along with all other IRQs).
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5950
> Signed-off-by: Andrzej Hajda
> Reviewed-by: Arun R Murthy
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 2 +-
> drivers/gpu/drm/i915/display/intel_hotplug.c | 11 ++-
> drivers/gpu/drm/i915/display/intel_hotplug.h | 2 +-
> drivers/gpu/drm/i915/i915_driver.c | 4 ++--
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> 5 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index f1c765ac7ab8aa..cd6139bb36151b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9022,7 +9022,7 @@ void intel_modeset_driver_remove_noirq(struct
> drm_i915_private *i915)
> intel_dp_mst_suspend(i915);
>
> /* MST is the last user of HPD work */
> - intel_hpd_cancel_work(i915);
> + intel_hpd_suspend(i915);
>
> /* poll work can call into fbdev, hence clean that up afterwards */
> intel_fbdev_fini(i915);
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
> b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 5f8b4f481cff9a..e1d384cb99df6b 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -303,6 +303,8 @@ static void i915_digport_work_func(struct work_struct
> *work)
> u32 old_bits = 0;
>
> spin_lock_irq(&dev_priv->irq_lock);
> + if (dev_priv->hotplug.suspended)
> + return spin_unlock_irq(&dev_priv->irq_lock);
> long_port_mask = dev_priv->hotplug.long_port_mask;
> dev_priv->hotplug.long_port_mask = 0;
> short_port_mask = dev_priv->hotplug.short_port_mask;
> @@ -353,6 +355,8 @@ void intel_hpd_trigger_irq(struct intel_digital_port
> *dig_port)
> struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>
> spin_lock_irq(&i915->irq_lock);
> + if (i915->hotplug.suspended)
> + return spin_unlock_irq(&i915->irq_lock);
> i915->hotplug.short_port_mask |= BIT(dig_port->base.port);
> spin_unlock_irq(&i915->irq_lock);
>
> @@ -475,6 +479,9 @@ void intel_hpd_irq_handler(struct drm_i915_private
> *dev_priv,
>
> spin_lock(&dev_priv->irq_lock);
>
> + if (dev_priv->hotplug.suspended)
> + return spin_unlock(&dev_priv->irq_lock);
> +
> /*
>* Determine whether ->hpd_pulse() exists for each pin, and
>* whether we have a short or a long pulse. This is needed
> @@ -603,6 +610,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>* just to make the assert_spin_locked checks happy.
>*/
> spin_lock_irq(&dev_priv->irq_lock);
> + dev_priv->hotplug.suspended = false;
> intel_hpd_irq_setup(dev_priv);
> spin_unlock_irq(&dev_priv->irq_lock);
> }
> @@ -721,13 +729,14 @@ void intel_hpd_init_work(struct drm_i915_private
> *dev_priv)
> intel_hpd_irq_storm_reenable_work);
> }
>
> -void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> +void intel_hpd_suspend(struct drm_i915_private *dev_priv)
> {
> if (!HAS_DISPLAY(dev_priv))
> return;
>
> spin_lock_irq(&dev_priv->irq_lock);
>
> + dev_priv->hotplug.suspended = true;
> dev_priv->hotplug.long_port_mask = 0;
> dev_priv->hotplug.short_port_mask = 0;
> dev_priv->hotplug.event_bits = 0;
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h
> b/drivers/gpu/drm/i915/display/intel_hotplug.h
> index b87e95d606e668..54bddc4dd63421 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> @@ -23,7 +23,7 @@ void intel_hpd_irq_handler(struct drm_i915_private
> *dev_priv,
> void intel_hpd_trigger_irq(struct intel_digital_port *dig_port);
> void intel_hpd_init(struct drm_i915_private *dev_priv);
> void intel_hpd_init_work(struct drm_i915_private *dev_priv);
> -void intel_hpd_cancel_work(str
Re: [PATCH v8 2/2] drm/gem: Don't map imported GEMs
On 8/16/22 22:55, Dmitry Osipenko wrote: > On 8/16/22 15:03, Christian König wrote: >> Am 16.08.22 um 13:44 schrieb Dmitry Osipenko: >>> [SNIP] The other complication I noticed is that we don't seem to keep around the fd after importing to a GEM handle. And I could imagine that doing so could cause issues with too many fd's. So I guess the best thing is to keep the status quo and let drivers that cannot mmap imported buffers just fail mmap? >>> That actually should be all the drivers excluding those that use >>> DRM-SHMEM because only DRM-SHMEM uses dma_buf_mmap(), that's why it >>> works for Panfrost. I'm pretty sure mmaping of imported GEMs doesn't >>> work for the MSM driver, isn't it? >>> >>> Intel and AMD drivers don't allow to map the imported dma-bufs. Both >>> refuse to do the mapping. >>> >>> Although, AMDGPU "succeeds" to do the mapping using >>> AMDGPU_GEM_DOMAIN_GTT, but then touching the mapping causes bus fault, >>> hence mapping actually fails. I think it might be the AMDGPU >>> driver/libdrm bug, haven't checked yet. >> >> That's then certainly broken somehow. Amdgpu should nerve ever have >> allowed to mmap() imported DMA-bufs and the last time I check it didn't. > > I'll take a closer look. So far I can only tell that it's a kernel > driver issue because once I re-applied this "Don't map imported GEMs" > patch, AMDGPU began to refuse mapping AMDGPU_GEM_DOMAIN_GTT. > >>> So we're back to the point that neither of DRM drivers need to map >>> imported dma-bufs and this was never tested. In this case this patch is >>> valid, IMO. > > Actually, I'm now looking at Etnaviv and Nouveau and seems they should > map imported dma-buf properly. I know that people ran Android on > Etnaviv. So maybe devices with a separated GPU/display need to map > imported display BO for Android support. Wish somebody who ran Android > on one of these devices using upstream drivers could give a definitive > answer. I may try to test Nouveau later on. > Nouveau+Intel combo doesn't work because of [1] that says: "Refuse to fault imported pages. This should be handled (if at all) by redirecting mmap to the exporter." [1] https://elixir.bootlin.com/linux/v5.19/source/drivers/gpu/drm/ttm/ttm_bo_vm.c#L154 Interestingly, I noticed that there are IGT tests which check prime mmaping of Nouveau+Intel [2] (added 9 years ago), but they fail as well, as expected. The fact that IGT has such tests is interesting because it suggests that the mapping worked in the past. It's also surprising that nobody cared to fix the failing tests. For the reference, I checked v5.18 and today's linux-next. [2] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/prime_nv_test.c#L132 Starting subtest: nv_write_i915_cpu_mmap_read Received signal SIGBUS. Stack trace: #0 [fatal_sig_handler+0x163] #1 [__sigaction+0x50] #2 [__igt_uniquereal_main354+0x406] #3 [main+0x23] #4 [__libc_start_call_main+0x80] #5 [__libc_start_main+0x89] #6 [_start+0x25] Subtest nv_write_i915_cpu_mmap_read: CRASH (0,005s) Starting subtest: nv_write_i915_gtt_mmap_read Received signal SIGBUS. Stack trace: #0 [fatal_sig_handler+0x163] #1 [__sigaction+0x50] #2 [__igt_uniquereal_main354+0x33d] #3 [main+0x23] #4 [__libc_start_call_main+0x80] #5 [__libc_start_main+0x89] #6 [_start+0x25] Subtest nv_write_i915_gtt_mmap_read: CRASH (0,004s) I'm curious about the Etnaviv driver because it uses own shmem implementation and maybe it has a working mmaping of imported GEMs since it imports the dma-buf pages into Entaviv BO. Although, it should be risking to map pages using a different caching attributes (WC) from the exporter, which is prohibited on ARM ad then one may try to map imported udmabuf. Apparently, the Intel DG TTM driver should be able to map imported dma-buf because it sets TTM_TT_FLAG_EXTERNAL_MAPPABLE. Overall, it still questionable to me whether it's worthwhile to allow the mmaping of imported GEMs since only Panfrost/Lima can do it out of all drivers and h/w that I tested. Feels like drivers that can do the mapping have it just because they can and not because they need. -- Best regards, Dmitry
[PATCH] drm/msm/dpu: drop unused memory allocation
Drop the dpu_cfg variable and corresponding kzalloc, which became unused
after changing hw catalog to static configuration.
Fixes: de7d480f5e8c ("drm/msm/dpu: make dpu hardware catalog static const")
Reported-by: kernel test robot
Reported-by: Dan Carpenter
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 0239a811d5ec..50ab17c9afd2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -1939,11 +1939,6 @@ static const struct dpu_mdss_hw_cfg_handler
cfg_handler[] = {
const struct dpu_mdss_cfg *dpu_hw_catalog_init(u32 hw_rev)
{
int i;
- struct dpu_mdss_cfg *dpu_cfg;
-
- dpu_cfg = kzalloc(sizeof(*dpu_cfg), GFP_KERNEL);
- if (!dpu_cfg)
- return ERR_PTR(-ENOMEM);
for (i = 0; i < ARRAY_SIZE(cfg_handler); i++) {
if (cfg_handler[i].hw_rev == hw_rev)
--
2.35.1
[PATCH] drm/msm/dpu: drop unused variable from dpu_kms_mdp_snapshot()
Follow up the merge of address fields and drop the variable that became
unused after the commit 9403f9a42c88 ("drm/msm/dpu: merge base_off with
blk_off in struct dpu_hw_blk_reg_map").
Fixes: 9403f9a42c88 ("drm/msm/dpu: merge base_off with blk_off in struct
dpu_hw_blk_reg_map")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 008e1420e6e5..1e1f45409aba 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -902,12 +902,10 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state
*disp_state, struct msm_k
int i;
struct dpu_kms *dpu_kms;
const struct dpu_mdss_cfg *cat;
- struct dpu_hw_mdp *top;
dpu_kms = to_dpu_kms(kms);
cat = dpu_kms->catalog;
- top = dpu_kms->hw_mdp;
pm_runtime_get_sync(&dpu_kms->pdev->dev);
--
2.35.1
Re: [PATCH] drm/msm: De-open-code some CP_EVENT_WRITE
On 21/08/2022 18:54, Rob Clark wrote: From: Rob Clark Signed-off-by: Rob Clark Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v4 4/6] clk: qcom: gpucc-sc7280: Add cx collapse reset support
On 19/08/2022 19:40, Akhil P Oommen wrote: Allow a consumer driver to poll for cx gdsc collapse through Reset framework. Signed-off-by: Akhil P Oommen --- (no changes since v3) Changes in v3: - Convert 'struct qcom_reset_ops cx_gdsc_reset' to 'static const' (Krzysztof) Changes in v2: - Minor update to use the updated custom reset ops implementation drivers/clk/qcom/gpucc-sc7280.c | 10 ++ 1 file changed, 10 insertions(+) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v4 3/6] clk: qcom: gdsc: Add a reset op to poll gdsc collapse
On 19/08/2022 19:40, Akhil P Oommen wrote: Add a reset op compatible function to poll for gdsc collapse. Signed-off-by: Akhil P Oommen Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v4 2/6] clk: qcom: Allow custom reset ops
On 19/08/2022 19:40, Akhil P Oommen wrote: Allow soc specific clk drivers to specify a custom reset operation. We will use this in an upcoming patch to allow gpucc driver to specify a differet reset operation for cx_gdsc. Signed-off-by: Akhil P Oommen Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH v6 2/4] drm/i915/fbdev: suspend HPD before fbdev unregistration
On Fri, Jul 22, 2022 at 02:51:41PM +0200, Andrzej Hajda wrote: > HPD event after fbdev unregistration can cause registration of deferred > fbdev which will not be unregistered later, causing use-after-free. > To avoid it HPD handling should be suspended before fbdev unregistration. > > It should fix following GPF: > [272.634530] general protection fault, probably for non-canonical address > 0x6b6b6b6b6b6b6b6b: [#1] PREEMPT SMP NOPTI > [272.634536] CPU: 0 PID: 6030 Comm: i915_selftest Tainted: G U > 5.18.0-rc5-CI_DRM_11603-g12dccf4f5eef+ #1 > [272.634541] Hardware name: Intel Corporation Raptor Lake Client > Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.2397.A01.2109300731 > 09/30/2021 > [272.634545] RIP: 0010:fb_do_apertures_overlap.part.14+0x26/0x60 > ... > [272.634582] Call Trace: > [272.634583] > [272.634585] do_remove_conflicting_framebuffers+0x59/0xa0 > [272.634589] remove_conflicting_framebuffers+0x2d/0xc0 > [272.634592] remove_conflicting_pci_framebuffers+0xc8/0x110 > [272.634595] drm_aperture_remove_conflicting_pci_framebuffers+0x52/0x70 > [272.634604] i915_driver_probe+0x63a/0xdd0 [i915] > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5329 > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5510 > Signed-off-by: Andrzej Hajda > Reviewed-by: Arun R Murthy > --- > drivers/gpu/drm/i915/display/intel_fbdev.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c > b/drivers/gpu/drm/i915/display/intel_fbdev.c > index 221336178991f0..94ddc0f34fde64 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c > @@ -573,7 +573,8 @@ void intel_fbdev_unregister(struct drm_i915_private > *dev_priv) > if (!ifbdev) > return; > > - cancel_work_sync(&dev_priv->fbdev_suspend_work); > + intel_fbdev_set_suspend(&dev_priv->drm, FBINFO_STATE_SUSPENDED, true); > + > if (!current_is_async()) > intel_fbdev_sync(ifbdev); > > @@ -618,7 +619,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int > state, bool synchronous > struct fb_info *info; > > if (!ifbdev || !ifbdev->vma) > - return; > + goto unlock; goto set_suspend; Reviewed-by: Imre Deak > > info = ifbdev->helper.fbdev; > > @@ -661,6 +662,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int > state, bool synchronous > drm_fb_helper_set_suspend(&ifbdev->helper, state); > console_unlock(); > > +unlock: > intel_fbdev_hpd_set_suspend(dev_priv, state); > } > > -- > 2.25.1 >
Re: [PATCH v6 1/4] drm/i915/hpd: postpone HPD cancel work after last user suspension
On Fri, Jul 22, 2022 at 02:51:40PM +0200, Andrzej Hajda wrote: > i915->hotplug.dig_port_work can be queued from intel_hpd_irq_handler > called by IRQ handler or by intel_hpd_trigger_irq called from dp_mst. > Since dp_mst is suspended after irq handler uninstall, a cleaner approach > is to cancel hpd work after intel_dp_mst_suspend, otherwise we risk > use-after-free. > > It should fix following WARNINGS: > [283.405824] cpu_latency_qos_update_request called for unknown object > [283.405866] WARNING: CPU: 2 PID: 240 at kernel/power/qos.c:296 > cpu_latency_qos_update_request+0x2d/0x100 > [283.405912] CPU: 2 PID: 240 Comm: kworker/u64:9 Not tainted > 5.18.0-rc6-Patchwork_103738v3-g1672d1c43e43+ #1 > [283.405915] Hardware name: Intel Corporation Raptor Lake Client > Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.2397.A01.2109300731 > 09/30/2021 > [283.405916] Workqueue: i915-dp i915_digport_work_func [i915] > [283.406020] RIP: 0010:cpu_latency_qos_update_request+0x2d/0x100 > ... > [283.406040] Call Trace: > [283.406041] > [283.406044] intel_dp_aux_xfer+0x60e/0x8e0 [i915] > [283.406131] ? finish_swait+0x80/0x80 > [283.406139] intel_dp_aux_transfer+0xc5/0x2b0 [i915] > [283.406218] drm_dp_dpcd_access+0x79/0x130 [drm_display_helper] > [283.406227] drm_dp_dpcd_read+0xe2/0xf0 [drm_display_helper] > [283.406233] intel_dp_hpd_pulse+0x134/0x570 [i915] > [283.406308] ? __down_killable+0x70/0x140 > [283.406313] i915_digport_work_func+0xba/0x150 [i915] > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4586 > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5558 > Signed-off-by: Andrzej Hajda > Reviewed-by: Arun R Murthy > --- > drivers/gpu/drm/i915/display/intel_display.c | 3 +++ > drivers/gpu/drm/i915/i915_irq.c | 1 - > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index a0f84cbe974fc3..f1c765ac7ab8aa 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -9021,6 +9021,9 @@ void intel_modeset_driver_remove_noirq(struct > drm_i915_private *i915) >*/ > intel_dp_mst_suspend(i915); > > + /* MST is the last user of HPD work */ > + intel_hpd_cancel_work(i915); > + MST still requires AUX and short HPD interrupts and during shutdown and suspend the order is suspend-MST -> disable-IRQs. So I think it makes more sense to move intel_dp_mst_suspend() to i915_driver_remove() before intel_irq_uninstall(). > /* poll work can call into fbdev, hence clean that up afterwards */ > intel_fbdev_fini(i915); > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 73cebc6aa65072..db14787aef95dd 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -4597,7 +4597,6 @@ void intel_irq_uninstall(struct drm_i915_private > *dev_priv) > > free_irq(irq, dev_priv); > > - intel_hpd_cancel_work(dev_priv); > dev_priv->runtime_pm.irqs_enabled = false; > } > > -- > 2.25.1 >
Re: [PATCH] drm/msm/dp: add atomic_check to bridge ops
On 22/08/2022 19:38, Abhinav Kumar wrote: Hi Dmitry On 8/22/2022 9:18 AM, Dmitry Baryshkov wrote: On 17/08/2022 21:01, Kuogee Hsieh wrote: DRM commit_tails() will disable downstream crtc/encoder/bridge if both disable crtc is required and crtc->active is set before pushing a new frame downstream. There is a rare case that user space display manager issue an extra screen update immediately followed by close DRM device while down stream display interface is disabled. This extra screen update will timeout due to the downstream interface is disabled but will cause crtc->active be set. Hence the followed commit_tails() called by drm_release() will pass the disable downstream crtc/encoder/bridge conditions checking even downstream interface is disabled. This cause the crash to happen at dp_bridge_disable() due to it trying to access the main link register to push the idle pattern out while main link clocks is disabled. This patch adds atomic_check to prevent the extra frame will not be pushed down if display interface is down so that crtc->active will not be set neither. This will fail the conditions checking of disabling down stream crtc/encoder/bridge which prevent drm_release() from calling dp_bridge_disable() so that crash at dp_bridge_disable() prevented. I must admit I had troubles parsing this description. However if I got you right, I think the check that the main link clock is running in the dp_bridge_disable() or dp_ctrl_push_idle() would be a better fix. Originally, thats what was posted https://patchwork.freedesktop.org/patch/496984/. This patch is also not so correct from my POV. It checks for the hpd status, while in reality it should check for main link clocks being enabled. Then it seemed like we were just protecting against an issue in the framework which was allowing the frames to be pushed even after the display was disconnected. The DP driver did send out the disconnect event correctly and as per the logs, this frame came down after that and the DRM fwk did allow it. So after discussing on IRC with Rob, we came up with this approach that if the display is not connected, then atomic_check should fail. That way the commit will not happen. Just seemed a bit cleaner instead of adding all our protections. The check to fail atomic_check if display is not connected seems out of place. In its current way it begs go to the upper layer, forbidding using disconnected sinks for all the drivers. There is nothing special in the MSM DP driver with respect to the HPD events processing and failing atomic_check() based on that. SError Interrupt on CPU7, code 0xbe000411 -- SError CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19 Hardware name: Google Lazor (rev3 - 8) (DT) pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : __cmpxchg_case_acq_32+0x14/0x2c lr : do_raw_spin_lock+0xa4/0xdc sp : ffc01092b6a0 x29: ffc01092b6a0 x28: 0028 x27: 0038 x26: 0004 x25: ffd2973dce48 x24: x23: x22: x21: ffd2978d0008 x20: ffd2978d0008 x19: ff80ff759fc0 x18: x17: 004800a501260460 x16: 0441043b04600438 x15: 0438089807d0 x14: 07b0089807800780 x13: x12: x11: 0438 x10: 07d0 x9 : ffd2973e09e4 x8 : ff8092d53300 x7 : ff808902e8b8 x6 : 0001 x5 : ff808902e880 x4 : x3 : ff80ff759fc0 x2 : 0001 x1 : x0 : ff80ff759fc0 Kernel panic - not syncing: Asynchronous SError Interrupt CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19 Hardware name: Google Lazor (rev3 - 8) (DT) Call trace: dump_backtrace.part.0+0xbc/0xe4 show_stack+0x24/0x70 dump_stack_lvl+0x68/0x84 dump_stack+0x18/0x34 panic+0x14c/0x32c nmi_panic+0x58/0x7c arm64_serror_panic+0x78/0x84 do_serror+0x40/0x64 el1h_64_error_handler+0x30/0x48 el1h_64_error+0x68/0x6c __cmpxchg_case_acq_32+0x14/0x2c _raw_spin_lock_irqsave+0x38/0x4c lock_timer_base+0x40/0x78 __mod_timer+0xf4/0x25c schedule_timeout+0xd4/0xfc __wait_for_common+0xac/0x140 wait_for_completion_timeout+0x2c/0x54 dp_ctrl_push_idle+0x40/0x88 dp_bridge_disable+0x24/0x30 drm_atomic_bridge_chain_disable+0x90/0xbc drm_atomic_helper_commit_modeset_disables+0x198/0x444 msm_atomic_commit_tail+0x1d0/0x374 commit_tail+0x80/0x108 drm_atomic_helper_commit+0x118/0x11c drm_atomic_commit+0xb4/0xe0 drm_client_modeset_commit_atomic+0x184/0x224 drm_client_modeset_commit_locked+0x58/0x160 drm_client_modeset_commit+0x3c/0x64 __drm_fb_helper_restore_fbdev_mode_unlocked+0x98/0xac drm_fb_helper_set_par+0x74/0x80 drm_fb_helper_hotplug_event+0xdc/0xe0 __drm_fb_helper_restore_fbdev_mode_unlocked+0x7c/0xac drm_fb_helper_restore_fbdev_mode_unlocked+0x20/0x2c drm_fb_helper_lastclose+0x20/0x2c drm_lastclose+0x44/0x6c drm_release+0x88/0xd4
Re: [PATCH] drm/msm/dp: add atomic_check to bridge ops
Hi Dmitry
On 8/22/2022 9:18 AM, Dmitry Baryshkov wrote:
On 17/08/2022 21:01, Kuogee Hsieh wrote:
DRM commit_tails() will disable downstream crtc/encoder/bridge if
both disable crtc is required and crtc->active is set before pushing
a new frame downstream.
There is a rare case that user space display manager issue an extra
screen update immediately followed by close DRM device while down
stream display interface is disabled. This extra screen update will
timeout due to the downstream interface is disabled but will cause
crtc->active be set. Hence the followed commit_tails() called by
drm_release() will pass the disable downstream crtc/encoder/bridge
conditions checking even downstream interface is disabled.
This cause the crash to happen at dp_bridge_disable() due to it trying
to access the main link register to push the idle pattern out while main
link clocks is disabled.
This patch adds atomic_check to prevent the extra frame will not
be pushed down if display interface is down so that crtc->active
will not be set neither. This will fail the conditions checking
of disabling down stream crtc/encoder/bridge which prevent
drm_release() from calling dp_bridge_disable() so that crash
at dp_bridge_disable() prevented.
I must admit I had troubles parsing this description. However if I got
you right, I think the check that the main link clock is running in the
dp_bridge_disable() or dp_ctrl_push_idle() would be a better fix.
Originally, thats what was posted
https://patchwork.freedesktop.org/patch/496984/.
Then it seemed like we were just protecting against an issue in the
framework which was allowing the frames to be pushed even after the
display was disconnected. The DP driver did send out the disconnect
event correctly and as per the logs, this frame came down after that and
the DRM fwk did allow it.
So after discussing on IRC with Rob, we came up with this approach that
if the display is not connected, then atomic_check should fail. That way
the commit will not happen.
Just seemed a bit cleaner instead of adding all our protections.
SError Interrupt on CPU7, code 0xbe000411 -- SError
CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
Hardware name: Google Lazor (rev3 - 8) (DT)
pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __cmpxchg_case_acq_32+0x14/0x2c
lr : do_raw_spin_lock+0xa4/0xdc
sp : ffc01092b6a0
x29: ffc01092b6a0 x28: 0028 x27: 0038
x26: 0004 x25: ffd2973dce48 x24:
x23: x22: x21: ffd2978d0008
x20: ffd2978d0008 x19: ff80ff759fc0 x18:
x17: 004800a501260460 x16: 0441043b04600438 x15: 0438089807d0
x14: 07b0089807800780 x13: x12:
x11: 0438 x10: 07d0 x9 : ffd2973e09e4
x8 : ff8092d53300 x7 : ff808902e8b8 x6 : 0001
x5 : ff808902e880 x4 : x3 : ff80ff759fc0
x2 : 0001 x1 : x0 : ff80ff759fc0
Kernel panic - not syncing: Asynchronous SError Interrupt
CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
Hardware name: Google Lazor (rev3 - 8) (DT)
Call trace:
dump_backtrace.part.0+0xbc/0xe4
show_stack+0x24/0x70
dump_stack_lvl+0x68/0x84
dump_stack+0x18/0x34
panic+0x14c/0x32c
nmi_panic+0x58/0x7c
arm64_serror_panic+0x78/0x84
do_serror+0x40/0x64
el1h_64_error_handler+0x30/0x48
el1h_64_error+0x68/0x6c
__cmpxchg_case_acq_32+0x14/0x2c
_raw_spin_lock_irqsave+0x38/0x4c
lock_timer_base+0x40/0x78
__mod_timer+0xf4/0x25c
schedule_timeout+0xd4/0xfc
__wait_for_common+0xac/0x140
wait_for_completion_timeout+0x2c/0x54
dp_ctrl_push_idle+0x40/0x88
dp_bridge_disable+0x24/0x30
drm_atomic_bridge_chain_disable+0x90/0xbc
drm_atomic_helper_commit_modeset_disables+0x198/0x444
msm_atomic_commit_tail+0x1d0/0x374
commit_tail+0x80/0x108
drm_atomic_helper_commit+0x118/0x11c
drm_atomic_commit+0xb4/0xe0
drm_client_modeset_commit_atomic+0x184/0x224
drm_client_modeset_commit_locked+0x58/0x160
drm_client_modeset_commit+0x3c/0x64
__drm_fb_helper_restore_fbdev_mode_unlocked+0x98/0xac
drm_fb_helper_set_par+0x74/0x80
drm_fb_helper_hotplug_event+0xdc/0xe0
__drm_fb_helper_restore_fbdev_mode_unlocked+0x7c/0xac
drm_fb_helper_restore_fbdev_mode_unlocked+0x20/0x2c
drm_fb_helper_lastclose+0x20/0x2c
drm_lastclose+0x44/0x6c
drm_release+0x88/0xd4
__fput+0x104/0x220
fput+0x1c/0x28
task_work_run+0x8c/0x100
do_exit+0x450/0x8d0
do_group_exit+0x40/0xac
__wake_up_parent+0x0/0x38
invoke_syscall+0x84/0x11c
el0_svc_common.constprop.0+0xb8/0xe4
do_el0_svc+0x8c/0xb8
el0_svc+0x2c/0x54
el0t_64_sync_handler+0x120/0x1c0
el0t_64_sync+0x190/0x194
SMP: stopping secondary CPUs
Kernel Offset: 0x128e80 from 0xffc00800
PHYS_OFFSET: 0x8000
CPU features: 0x800,00c2a015,19801c82
Memory Limit: none
Fixes: 8a3b4c17f863 ("drm/msm/dp: employ
Re: [RFC] drm/bridge: adv7533: remove dynamic lane switching from adv7533 bridge
On 09/08/2022 22:40, Laurent Pinchart wrote:
Hi Abhinav,
Thank you for the patch.
On Mon, Aug 08, 2022 at 05:35:30PM -0700, Abhinav Kumar wrote:
adv7533 bridge tries to dynamically switch lanes based on the
mode by detaching and attaching the mipi dsi device.
This approach is incorrect because as per the DSI spec the
number of lanes is fixed at the time of system design or initial
configuration and may not change dynamically.
Is that really so ? The number of lanes connected on the board is
certainlyset at design time, but a lower number of lanes can be used at
runtime. It shouldn't change dynamically while the display is on, but it
could change at mode set time.
I'm not sure if I interpreted the standard correctly, but I tended to
have the same interpretation as Abhinav did.
Anyway, even if we allow the drivers to switch the amount of lanes, this
should not happen in the form of detach()/attach() cycle. The drivers
use mipi_dsi_attach() as way to signal the DSI hosts that the sink
device is ready. Some of DSI hosts (including MSM one) would bind
components from the attach callback.
If we were to support dynamically changing the amount of lanes, I would
ask for additional mipi_dsi API call telling the host to switch the
amount of lanes. And note that this can open the can of worms. Different
hosts might have different requirements here. For example for the MSM
platform the amount of lanes is programmed during bridge_pre_enable
chain call, so it is possible to just set the amount of lanes following
the msm_dsi_device::lanes. Other hosts might have other requirements.
Thus said I'd suggest accepting this patch and maybe working on the
API/support for the lane switching as a followup.
In addition this method of dynamic switch of detaching and
attaching the mipi dsi device also results in removing
and adding the component which is not necessary.
Yes, that doesn't look good, and the .mode_valid() operation is
definitely not the right point where to set the number of lanes.
.mode_set()?
This approach is also prone to deadlocks. So for example, on the
db410c whenever this path is executed with lockdep enabled,
this results in a deadlock due to below ordering of locks.
Even without the deadlock, we are pulling the whole DRM device from
beneath the DSI panel by unbinding all the components. This is not going
to work correctly.
-> #1 (crtc_ww_class_acquire){+.+.}-{0:0}:
lock_acquire+0x6c/0x90
drm_modeset_acquire_init+0xf4/0x150
drmm_mode_config_init+0x220/0x770
msm_drm_bind+0x13c/0x654
try_to_bring_up_aggregate_device+0x164/0x1d0
__component_add+0xa8/0x174
component_add+0x18/0x2c
dsi_dev_attach+0x24/0x30
dsi_host_attach+0x98/0x14c
devm_mipi_dsi_attach+0x38/0xb0
adv7533_attach_dsi+0x8c/0x110
adv7511_probe+0x5a0/0x930
i2c_device_probe+0x30c/0x350
really_probe.part.0+0x9c/0x2b0
__driver_probe_device+0x98/0x144
driver_probe_device+0xac/0x14c
__device_attach_driver+0xbc/0x124
bus_for_each_drv+0x78/0xd0
__device_attach+0xa8/0x1c0
device_initial_probe+0x18/0x24
bus_probe_device+0xa0/0xac
deferred_probe_work_func+0x90/0xd0
process_one_work+0x28c/0x6b0
worker_thread+0x240/0x444
kthread+0x110/0x114
ret_from_fork+0x10/0x20
-> #0 (component_mutex){+.+.}-{3:3}:
__lock_acquire+0x1280/0x20ac
lock_acquire.part.0+0xe0/0x230
lock_acquire+0x6c/0x90
__mutex_lock+0x84/0x400
mutex_lock_nested+0x3c/0x70
component_del+0x34/0x170
dsi_dev_detach+0x24/0x30
dsi_host_detach+0x20/0x64
mipi_dsi_detach+0x2c/0x40
adv7533_mode_set+0x64/0x90
adv7511_bridge_mode_set+0x210/0x214
drm_bridge_chain_mode_set+0x5c/0x84
crtc_set_mode+0x18c/0x1dc
drm_atomic_helper_commit_modeset_disables+0x40/0x50
msm_atomic_commit_tail+0x1d0/0x6e0
commit_tail+0xa4/0x180
drm_atomic_helper_commit+0x178/0x3b0
drm_atomic_commit+0xa4/0xe0
drm_client_modeset_commit_atomic+0x228/0x284
drm_client_modeset_commit_locked+0x64/0x1d0
drm_client_modeset_commit+0x34/0x60
drm_fb_helper_lastclose+0x74/0xcc
drm_lastclose+0x3c/0x80
drm_release+0xfc/0x114
__fput+0x70/0x224
fput+0x14/0x20
task_work_run+0x88/0x1a0
do_exit+0x350/0xa50
do_group_exit+0x38/0xa4
__wake_up_parent+0x0/0x34
invoke_syscall+0x48/0x114
el0_svc_common.constprop.0+0x60/0x11c
do_el0_svc+0x30/0xc0
el0_svc+0x58/0x100
el0t_64_sync_handler+0x1b0/0x1bc
el0t_64_sync+0x18c/0x190
Due to above reasons, remove the dynamic lane switching
code from adv7533 bridge chip and filter out the modes
which would need different number of lane
Re: [PATCH] drm/msm/dp: add atomic_check to bridge ops
On 17/08/2022 21:01, Kuogee Hsieh wrote:
DRM commit_tails() will disable downstream crtc/encoder/bridge if
both disable crtc is required and crtc->active is set before pushing
a new frame downstream.
There is a rare case that user space display manager issue an extra
screen update immediately followed by close DRM device while down
stream display interface is disabled. This extra screen update will
timeout due to the downstream interface is disabled but will cause
crtc->active be set. Hence the followed commit_tails() called by
drm_release() will pass the disable downstream crtc/encoder/bridge
conditions checking even downstream interface is disabled.
This cause the crash to happen at dp_bridge_disable() due to it trying
to access the main link register to push the idle pattern out while main
link clocks is disabled.
This patch adds atomic_check to prevent the extra frame will not
be pushed down if display interface is down so that crtc->active
will not be set neither. This will fail the conditions checking
of disabling down stream crtc/encoder/bridge which prevent
drm_release() from calling dp_bridge_disable() so that crash
at dp_bridge_disable() prevented.
I must admit I had troubles parsing this description. However if I got
you right, I think the check that the main link clock is running in the
dp_bridge_disable() or dp_ctrl_push_idle() would be a better fix.
SError Interrupt on CPU7, code 0xbe000411 -- SError
CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
Hardware name: Google Lazor (rev3 - 8) (DT)
pstate: a04000c9 (NzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __cmpxchg_case_acq_32+0x14/0x2c
lr : do_raw_spin_lock+0xa4/0xdc
sp : ffc01092b6a0
x29: ffc01092b6a0 x28: 0028 x27: 0038
x26: 0004 x25: ffd2973dce48 x24:
x23: x22: x21: ffd2978d0008
x20: ffd2978d0008 x19: ff80ff759fc0 x18:
x17: 004800a501260460 x16: 0441043b04600438 x15: 0438089807d0
x14: 07b0089807800780 x13: x12:
x11: 0438 x10: 07d0 x9 : ffd2973e09e4
x8 : ff8092d53300 x7 : ff808902e8b8 x6 : 0001
x5 : ff808902e880 x4 : x3 : ff80ff759fc0
x2 : 0001 x1 : x0 : ff80ff759fc0
Kernel panic - not syncing: Asynchronous SError Interrupt
CPU: 7 PID: 3878 Comm: Xorg Not tainted 5.19.0-stb-cbq #19
Hardware name: Google Lazor (rev3 - 8) (DT)
Call trace:
dump_backtrace.part.0+0xbc/0xe4
show_stack+0x24/0x70
dump_stack_lvl+0x68/0x84
dump_stack+0x18/0x34
panic+0x14c/0x32c
nmi_panic+0x58/0x7c
arm64_serror_panic+0x78/0x84
do_serror+0x40/0x64
el1h_64_error_handler+0x30/0x48
el1h_64_error+0x68/0x6c
__cmpxchg_case_acq_32+0x14/0x2c
_raw_spin_lock_irqsave+0x38/0x4c
lock_timer_base+0x40/0x78
__mod_timer+0xf4/0x25c
schedule_timeout+0xd4/0xfc
__wait_for_common+0xac/0x140
wait_for_completion_timeout+0x2c/0x54
dp_ctrl_push_idle+0x40/0x88
dp_bridge_disable+0x24/0x30
drm_atomic_bridge_chain_disable+0x90/0xbc
drm_atomic_helper_commit_modeset_disables+0x198/0x444
msm_atomic_commit_tail+0x1d0/0x374
commit_tail+0x80/0x108
drm_atomic_helper_commit+0x118/0x11c
drm_atomic_commit+0xb4/0xe0
drm_client_modeset_commit_atomic+0x184/0x224
drm_client_modeset_commit_locked+0x58/0x160
drm_client_modeset_commit+0x3c/0x64
__drm_fb_helper_restore_fbdev_mode_unlocked+0x98/0xac
drm_fb_helper_set_par+0x74/0x80
drm_fb_helper_hotplug_event+0xdc/0xe0
__drm_fb_helper_restore_fbdev_mode_unlocked+0x7c/0xac
drm_fb_helper_restore_fbdev_mode_unlocked+0x20/0x2c
drm_fb_helper_lastclose+0x20/0x2c
drm_lastclose+0x44/0x6c
drm_release+0x88/0xd4
__fput+0x104/0x220
fput+0x1c/0x28
task_work_run+0x8c/0x100
do_exit+0x450/0x8d0
do_group_exit+0x40/0xac
__wake_up_parent+0x0/0x38
invoke_syscall+0x84/0x11c
el0_svc_common.constprop.0+0xb8/0xe4
do_el0_svc+0x8c/0xb8
el0_svc+0x2c/0x54
el0t_64_sync_handler+0x120/0x1c0
el0t_64_sync+0x190/0x194
SMP: stopping secondary CPUs
Kernel Offset: 0x128e80 from 0xffc00800
PHYS_OFFSET: 0x8000
CPU features: 0x800,00c2a015,19801c82
Memory Limit: none
Fixes: 8a3b4c17f863 ("drm/msm/dp: employ bridge mechanism for display enable and
disable")
Reported-by: Leonard Lausen
Suggested-by: Rob Clark
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/17
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_drm.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 6df25f7..c682588 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -31,6 +31,25 @@ static enum drm_connector_status dp_bridge_detect(struct
drm_bridge *bridge)
connector_status_disconnected;
}
+static int dp_bridge_atomic_check(struct
[PATCH 0/2] drm/rockchip: dw_hdmi: Add 4k@30 support
This series adds support for 4k@30 to the rockchip HDMI controller. This has been tested on a rk3568 rock3a board. It should be possible to add 4k@60 support the same way, but it doesn't work for me, so let's add 4k@30 as a first step. Sascha Sascha Hauer (2): drm/rockchip: dw_hdmi: relax mode_valid hook drm/rockchip: dw_hdmi: Add support for 4k@30 resolution drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) -- 2.30.2
[PATCH 1/2] drm/rockchip: dw_hdmi: relax mode_valid hook
The driver checks if the pixel clock of the given mode matches an entry
in the mpll config table. The frequencies in the mpll table are meant as
a frequency range up to which the entry works, not as a frequency that
must match the pixel clock. Return MODE_OK when the pixelclock is
smaller than one of the mpll frequencies to allow for more display
resolutions.
Signed-off-by: Sascha Hauer
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index c14f888938688..b6b662dabedc6 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -251,7 +251,7 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *hdmi, void
*data,
int i;
for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
- if (pclk == mpll_cfg[i].mpixelclock) {
+ if (pclk <= mpll_cfg[i].mpixelclock) {
valid = true;
break;
}
--
2.30.2
[PATCH 2/2] drm/rockchip: dw_hdmi: Add support for 4k@30 resolution
This adds the PLL/phy settings to support higher resolutions like 4k@30.
The values were taken from the Rockchip downstream Kernel.
Signed-off-by: Sascha Hauer
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index b6b662dabedc6..5d1cc36406aca 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -160,6 +160,12 @@ static const struct dw_hdmi_mpll_config
rockchip_mpll_cfg[] = {
{ 0x214c, 0x0003},
{ 0x4064, 0x0003}
},
+ }, {
+ 34000, {
+ { 0x0052, 0x0003 },
+ { 0x214d, 0x0003 },
+ { 0x4065, 0x0003 },
+ },
}, {
~0UL, {
{ 0x00a0, 0x000a },
@@ -185,6 +191,8 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = {
14625, { 0x0038, 0x0038, 0x0038 },
}, {
14850, { 0x, 0x0038, 0x0038 },
+ }, {
+ 6, { 0x, 0x, 0x },
}, {
~0UL, { 0x, 0x, 0x},
}
--
2.30.2
Re: [PATCH v3 5/5] drm: rcar-du: dsi: Fix VCLKSET write
Hi Tomi, Thank you for the patch. On Mon, Aug 22, 2022 at 05:34:01PM +0300, Tomi Valkeinen wrote: > From: Tomi Valkeinen > > rcar_mipi_dsi_startup() writes correct values to VCLKSET, but as it uses > or-operation to add the new values to the current value in the register, > it should first make sure the fields are cleared. > > Do this by using rcar_mipi_dsi_write() to write the VCLKSET_CKEN bit to > VCLKSET before the rest of the VCLKSET configuration. > > Signed-off-by: Tomi Valkeinen > --- > drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > index 06250f2f3499..b60a6d4a5d46 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c > @@ -412,9 +412,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi > *dsi, > return ret; > } > > - /* Enable DOT clock */ > - vclkset = VCLKSET_CKEN; > - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); > + /* Clear VCLKSET and enable DOT clock */ > + rcar_mipi_dsi_write(dsi, VCLKSET, VCLKSET_CKEN); > + > + vclkset = 0; > > if (dsi_format == 24) > vclkset |= VCLKSET_BPP_24; You can replace one more set() with a write(): diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c index 06250f2f3499..2744ea23e6f6 100644 --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c @@ -414,7 +414,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, /* Enable DOT clock */ vclkset = VCLKSET_CKEN; - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); + rcar_mipi_dsi_write(dsi, VCLKSET, vclkset); if (dsi_format == 24) vclkset |= VCLKSET_BPP_24; @@ -429,7 +429,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi, vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div) | VCLKSET_LANE(dsi->lanes - 1); - rcar_mipi_dsi_set(dsi, VCLKSET, vclkset); + rcar_mipi_dsi_write(dsi, VCLKSET, vclkset); /* After setting VCLKSET register, enable VCLKEN */ rcar_mipi_dsi_set(dsi, VCLKEN, VCLKEN_CKEN); I'll apply patches 1/5 to 4/5 to my tree already. -- Regards, Laurent Pinchart
