Re: [Freedreno] [RFC PULL] Add Display Support for Qualcomm SDM845

[Jordan Crouse <jcrouse@codeaurora.org>]

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@vger.kernel.org. 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@vmware.com> and Lukasz Spintzyk <lukasz.spintzyk@displaylink.com>
> - Uma Shankar <uma.shankar@intel.com> 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