On Tue, Feb 13, 2018 at 02:18:13PM -0500, Sean Paul wrote: > Hi dri-devel, > Qualcomm has been working for the past few weeks on forward porting their > downstream drm driver from 4.14 to mainline. Please consider this PR as a > request for review, rather than an attempt at mainlining the code as it > currently stands. The goal is get this driver in shape over the next coming > months. > > In the meantime, I'll be hosting a tree here [1] to stage the fixes. Patches > will be posted and reviewed on linux-arm-msm@xxxxxxxxxxxxxxx. Once things look > good, I'll send another pull 4realz. > > To get the ball rolling, I've done some review on the new connector code, my > comments are below. > > Thanks in advance for your constructive feedback :) > > Sean > > [1]- git://people.freedesktop.org/~seanpaul/dpu-staging > > Review feedback: > ---------------- > - Solve the splash screen handling (or remove it) > - Simplify devicetree binding (remove register offsets) > feedback from reviewing sde_connector.c: > - Rationalize backlight implementation in sde_connector (display_count static) > - Sort out the dsi event passing between dsi/encoder/connector (move to encoder) > - include/uapi/drm/msm_drm_pp.h needs opensource userspace (or removal) > - connector->state access violations reading/writing mode_info > - s/sde_rect/drm_rect/ > - sde_kms_info usage needs to be replaced with formal data structures (not > stringified keypairs) > - sde_connector_ops needs to be trimmed, duplicates connector helpers, info > hooks circumvent state, and other hooks should be stored in state or > prepopulated (get_dst_format) > - sde_connector_get_dpms unused > - esd status check should migrate to encoder from connector > - backlight should be handled in panel drivers, not in the generic connector/dsi > encoder > - sde_connector_helper_bridge_disable is called from encoder and calls back into > set_power encoder function. if backlight, and esd status are removed, > pre_kickoff can probably go away > - sde_connector_clk_ctrl is another example of encoder->connector->encoder call > - RETIRE_FENCE connector property should be removed, opting for the native > atomic fences > - ROI (regions of interest) should be expressed per-plane instead of connector. > there is work ongoing to support dirty_rects per-plane by Deepak Singh Rawat > <drawat@xxxxxxxxxx> and Lukasz Spintzyk <lukasz.spintzyk@xxxxxxxxxxxxxxx> > - Uma Shankar <uma.shankar@xxxxxxxxx> has proposed HDR source metadata > properties on the list, we should pivot to those instead of hand-rolling them > in the sde driver > - Convert HDCP implementation to upstream Content Protection property > - Merge dsi and dsi_staging into one driver > - Writeback connector has been proposed by ARM (Liviu Dudau and Brian Starkey), > we should work with their proposal instead of rolling OUT_FB ourselves > - sde_connector_set_property should be replaced with atomic helper > - dsi hotplug can probably be punted to the panel driver > - dpms should switch to enable/disable (or at least use the atomic helpers) > - dsi mode handling should also defer to the panel driver > - SDE_WB_CONFIG ioctl should be removed in favor of the existing ioctl to add > user-defined modes > - dp implementation should use the existing dp helpers wherever possible > - lots of duplicated structures in dsi_defs.h that can be replaced with existing > drm structs > - mode_valid should be split up and implemented directly in connector/encoder as > appropriate > - sde_connector->aspace seems like it's unused? > > > The following changes since commit 9afe236df559d0dc6818f64e728a3f931a0a2231: > > drm/msm/dsi: Fix potential NULL pointer dereference in msm_dsi_modeset_init (2018-02-12 10:25:15 -0500) > > are available in the Git repository at: > > git://people.freedesktop.org/~seanpaul/dpu-staging for-next-compiles > > for you to fetch changes up to 672005da148f82021a62d4fa658728e19f13097e: > > ARM: dts: msm: add device tree changes for SDM845 (2018-02-13 13:14:43 -0500) > > ---------------------------------------------------------------- > Jeykumar Sankaran (9): > dt-bindings: msm/dsi: Add mdp transfer time to msm dsi binding > dt-bindings: msm/disp: Add bindings for Snapdragon 845 DPU > drm: Core changes > drm/msm: add DPU DRM driver to support SDM845 > drm/msm: Change mdp_get_format arguments > drm/msm: Core msm changes > drm/msm: Add DSI Staging driver > drm/msm: Add DisplayPort support > ARM: dts: msm: add device tree changes for SDM845 > > Manasi Navare (1): > drm/dp: Add HBR3 support in existing DRM DP helpers > > Rob Clark (1): > drm/msm: rename mdp->disp > > drivers/gpu/drm/msm/dpu_io_util.c | 507 ++ > include/linux/dpu_io_util.h | 113 + This looks like it was borrowed from some older downstream code - its mostly a wrapper for iomem and clock operations and a goodly chuck of dead i2c and gpio code too. The header definitely doesn't need to be in include/linux and we might be able to save a bit of space by merging the iomem wrappers with the code we already have. Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
