From: Jani Nikula <jani.nikula@intel.com>
To: "Lin, Wayne" <Wayne.Lin@amd.com>, "dri-devel@lists.freedesktop.org"
 <dri-devel@lists.freedesktop.org>, "amd-gfx@lists.freedesktop.org"
 <amd-gfx@lists.freedesktop.org>
Subject: RE: [PATCH v4] drm/dp_mst: Clear MSG_RDY flag before sending new
 message
In-Reply-To: <CO6PR12MB548948F83852B367F228F9A9FC51A@CO6PR12MB5489.namprd12.prod.outlook.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
References: <20230608114316.3078024-1-Wayne.Lin@amd.com>
 <87a5xarvh9.fsf@intel.com>
 <CO6PR12MB548948F83852B367F228F9A9FC51A@CO6PR12MB5489.namprd12.prod.outlook.com>
Date: Fri, 09 Jun 2023 11:53:17 +0300
Message-ID: <871qilrp8i.fsf@intel.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list
Cc: "stable@vger.kernel.org" <stable@vger.kernel.org>, "Zuo,
 Jerry" <Jerry.Zuo@amd.com>
Errors-To: dri-devel-bounces@lists.freedesktop.org
Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>

>> > bool *handled)
>> > +int drm_dp_mst_hpd_irq_handle_event(struct drm_dp_mst_topology_mgr
>> *mgr, const u8 *esi,
>> > +                               u8 *ack, bool *handled)
>> >  {
>> >     int ret = 0;
>> >     int sc;
>> > @@ -4078,18 +4089,47 @@ int drm_dp_mst_hpd_irq(struct
>> drm_dp_mst_topology_mgr *mgr, u8 *esi, bool *handl
>> >     if (esi[1] & DP_DOWN_REP_MSG_RDY) {
>> >             ret = drm_dp_mst_handle_down_rep(mgr);
>> >             *handled = true;
>> > +           *ack |= DP_DOWN_REP_MSG_RDY;
>>
>> My idea was that esi and ack would be the same size buffers, so the caller
>> wouldn't have to worry where exactly to point ack to.
>>
>> I think the asymmetry here is misleading, with ack and esi having to point at
>> different locations.
>>
> Thanks, Jani.
>
> But Event status Indicator Files (DPCD 0x2000h ~ 0x21FFH) are not all designed
> to be ack clear, e.g. esi[0] here. My thought is to be precise about what is handled
> and what is going to be ack clear. Otherwise, write ack[0] to DPCD 0x2002h is
> not reasonable.

The point is that you have the same indexes everywhere, even if ack[0]
ends up being unused.

Handle esi[1] & DP_DOWN_REP_MSG_RDY, set ack[1] |= DP_DOWN_REP_MSG_RDY.

Similar pattern everywhere, drm core and drivers. The only place that
needs to know the difference is where the ack is written back to DPCD.

If we end up adding more helpers for drm core handling ESI, we'll keep
repeating the same pattern, instead of passing individual u8 acks
everywhere, with the driver having to figure out what pointers to pass.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center