lkml.org 
[lkml]   [2022]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus
    Hi,

    On Sat, Apr 2, 2022 at 3:37 AM Dmitry Baryshkov
    <dmitry.baryshkov@linaro.org> wrote:
    >
    > On 01/04/2022 02:22, Doug Anderson wrote:
    > > Hi,
    > >
    > > On Wed, Mar 30, 2022 at 9:03 AM Sankeerth Billakanti
    > > <quic_sbillaka@quicinc.com> wrote:
    > >>
    > >> @@ -1547,6 +1593,10 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
    > >>
    > >> dp_display->encoder = encoder;
    > >>
    > >> + ret = dp_display_get_next_bridge(dp_display);
    > >> + if (ret)
    > >> + return ret;
    > >
    > > It feels weird to me that this is in a function called "modeset_init",
    > > though I certainly don't know the structure of the MSM display code
    > > well enough to fully comment.
    >
    > It's called modeset_init() as it initializes KMS objects used by DP
    > driver. We have similar functions for dsi and hdmi

    Sorry, I wasn't meaning to imply that modeset_init() was a bad name or
    anything. Mostly saying that I wasn't sure that modeset init was the
    proper time to populate the aux bus. ...but then again, perhaps it is
    given the current structure of this driver?


    > > My expectation would have been that
    > > devm_of_dp_aux_populate_ep_devices() would have been called from your
    > > probe routine and then you would have returned -EPROBE_DEFER from your
    > > probe if you were unable to find the panel afterwards.
    >
    > I don't think it's possible to call it from probe() since
    > drm_dp_aux_register() is called only from dp_display_bind().
    > The PHY also isn't initialized at that moment, so we can not probe AUX
    > devices.
    >
    > The overall semantics of the AUX bus is not clear to me.
    > Typically the bus is populated (and probed) when devices are accessible.
    > But for the display related buses this might not be the case.

    In general the AUX bus is modeled much like the i2c bus. You probe the
    sub-device when you're able to transfer. Then you can confirm that the
    device is actually there and init the device.


    > For example for the DSI bus we clearly define that DSI transfer are not
    > possible before the corresponding bridge's (or panel's) enable call.
    >
    > Maybe the same approach should be adopted for the AUX bus. This would
    > allow us to populate the AUX bus before hardware access is actually
    > possible, thus creating all the DRM bridges before the hardware is
    > actually up and running.

    So I guess what you're proposing is that you could probe the devices
    under the AUX bus and they could acquire resources (and possibly
    return EPROBE_DEFER) at a point in time _before_ it's actually
    possible to transfer. Then I guess you'd later do the transfer?

    I guess conceivably one could re-design the DRM subsystem like that,
    but I don't think it's trivial. Why? I believe that we need to know
    things about the panel at probe time. For instance, we need to be able
    to populate the panel's modes. To get this information we need the
    EDID which means we need to be able to do a transfer. If we're using
    an AUX backlight we also need to add info about the backlight at probe
    time and that also needs the transfer to work.


    So I guess the net result is maybe we should just keep it where it is.
    Long term I'd be interested in knowing if there's a reason why we
    can't structure the driver so that AUX transfers can happen with less
    intertwining with the rest of the code, but that can happen later. I
    would expect that you'd basically just need clocks and regulators on
    and maybe your PHY on. Ideally with some pm_runtime fun we should be
    able to do that independently with anything else the driver needs to
    do?

    -Doug

    \
     
     \ /
      Last update: 2022-04-02 19:08    [W:4.151 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site